Skip to content

feat(#281): flexible roles helper#302

Open
MichalBryxi wants to merge 1 commit into
CrowdStrike:mainfrom
MichalBryxi:feat/281-rules-format
Open

feat(#281): flexible roles helper#302
MichalBryxi wants to merge 1 commit into
CrowdStrike:mainfrom
MichalBryxi:feat/281-rules-format

Conversation

@MichalBryxi

Copy link
Copy Markdown

Supported format:

  • Array of strings
  • Concatenated string
  • Multiple arguments


roles('#role6', '#role7', function() {
it('works', function() {
assert.ok(this.role.match(/role[6|7]/));

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really not sure whether there might be a better way to do this? For mocha there might be a solution in manually tracking number of assertions like here: https://github.com/mochajs/mocha/wiki/HOW-TO:-Count-assertions Does that make sense? Do we want it?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coming back to this the overall number of assertions expected is covered in the calling test: https://github.com/MichalBryxi/faltest/blob/6c3d9c9358a56c33cb5027e4bfe00aabbd47ca8b/packages/mocha/test/acceptance/index-test.js#L113

And here we match against the role names.

So we verify that correct number of cases that we expect to be executed are executed. And we verify that the correct ones are being executed. So I believe that this should be ok.

Comment thread packages/mocha/src/role.js
Comment thread packages/mocha/src/role.js Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread packages/mocha/src/role.js

roles('#role6', '#role7', function() {
it('works', function() {
assert.ok(this.role.match(/role[6|7]/));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea.

@MichalBryxi MichalBryxi force-pushed the feat/281-rules-format branch 3 times, most recently from bb5d19b to 8104c36 Compare June 26, 2020 22:08
@MichalBryxi

Copy link
Copy Markdown
Author

I have problem figuring out what is the problem on Travis: https://travis-ci.org/github/CrowdStrike/faltest/jobs/702527287

For me the result is ok:

~/Projects/faltest feat/281-rules-format
❯ yarn test:acceptance
yarn run v1.17.3
$ mocha --recursive


  Test | Acceptance.js
@faltest/cli v2.4.0


  Fixtures | Retries
    ✓ works (98ms)


  1 passing (2s)

    ✓ faltest --retries 1 fixtures/retries-test.js (3081ms)

  Test | Examples.js
    /Users/mbryxi/Projects/faltest/examples/custom-cli
$ yarn my-cli
$ /Users/mbryxi/Projects/faltest/node_modules/.bin/my-cli
@faltest/cli v2.4.0


  sample
    ✓ works (1918ms)


  1 passing (3s)

      ✓ yarn start (4174ms)
    /Users/mbryxi/Projects/faltest/examples/full-suite
$ faltest --failure-artifacts --failure-artifacts-output-dir dist --target fixtures --env dev --tag user --tag smoke
@faltest/cli v2.4.0


  sample
    #user
      ✓ works #smoke


  1 passing (3s)

      ✓ yarn start --target fixtures --env dev --tag user --tag smoke (4364ms)
$ faltest --failure-artifacts --failure-artifacts-output-dir dist --tag admin --tag !smoke
@faltest/cli v2.4.0


  sample
    #admin
      ✓ shows existing feature | flags: finished-feature
      - shows new feature | flags: in-progress-feature
      ✓ hides unfinished feature | flags: !in-progress-feature


  2 passing (11s)
  1 pending

      ✓ yarn start --tag admin --tag !smoke (11873ms)
$ faltest --failure-artifacts --failure-artifacts-output-dir dist --tag admin --filter unfinished
@faltest/cli v2.4.0


  sample
    #admin
      ✓ hides unfinished feature | flags: !in-progress-feature


  1 passing (3s)

      ✓ yarn start --tag admin --filter unfinished (4527ms)
    /Users/mbryxi/Projects/faltest/examples/lifecycle-only
$ mocha tests/**/*-test.js --exit


  sample
    ✓ works (1848ms)


  1 passing (3s)

      ✓ yarn start (3593ms)
    /Users/mbryxi/Projects/faltest/examples/multiple-browsers
$ faltest tests/**/*-test.js --browsers 2
@faltest/cli v2.4.0


  sample
    ✓ works (1947ms)


  1 passing (4s)

      ✓ yarn start (5722ms)
    /Users/mbryxi/Projects/faltest/examples/runner-only
$ faltest tests/**/*-test.js
@faltest/cli v2.4.0


  sample
    ✓ works (1900ms)


  1 passing (3s)

      ✓ yarn start (3995ms)


  8 passing (41s)

✨  Done in 41.64s.

@MichalBryxi

MichalBryxi commented Jun 26, 2020

Copy link
Copy Markdown
Author

Running the whole yarn test locally returns zero exit code for me: https://gist.github.com/MichalBryxi/f107319d27cde38cfffacd6815f587e8

Any help @kellyselden ?

@MichalBryxi MichalBryxi force-pushed the feat/281-rules-format branch 4 times, most recently from 68eacd2 to abe8ba8 Compare January 23, 2021 17:50
Supported format:
 - Concatenated string
 - Multiple arguments
@MichalBryxi MichalBryxi force-pushed the feat/281-rules-format branch from abe8ba8 to 10462dc Compare January 23, 2021 23:05
@MichalBryxi

MichalBryxi commented Jan 23, 2021

Copy link
Copy Markdown
Author

Figured out that it was just about the version of node used, because I used matchAll that is available from node-12.x. Oh well. Since this one has been open for a while: Is this PR still relevant? i.e: Merge or close?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants