-
Notifications
You must be signed in to change notification settings - Fork 69
fix: tags no operator #1184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: tags no operator #1184
Conversation
commit: |
test/src/test-reader/test-parser.js
Outdated
| }); | ||
|
|
||
| describe("if set", () => { | ||
| describe.only("if set", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
describe.only!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
| ]), | ||
| parent: { | ||
| fullTitle: () => "Some parent", | ||
| tags: new Map([]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be Set?
Also, as far as I can tell, the bug was that if parent has tag "smoke" and we ran testplane with !smoke, the test would still launch, because it was enough to get ok only from children. But here you just add an empty array, is this really sufficient to test the case we uncovered now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we use map for understanding dynamic tag or not, in key we have a tag name in value true or false
| } else { | ||
| current = current.parent; | ||
| } | ||
| [...current.tags.keys()].forEach(item => allTags.add(item)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why keys()? I know it's an alias in set for values, or we actually use Map for tags? It's becoming hard to follow, i think we should use either Map or Set everywhere for tags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
answered above
No description provided.