-
Notifications
You must be signed in to change notification settings - Fork 130
Report skipped hermione tests #697
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
Conversation
add missing skipped spec fixture
| describe("with skip", () => { | ||
| it.skip("should be skipped", () => {}); | ||
| }); | ||
|
|
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.
maybe we also need fixture for the describe.skip?
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.
They actually have problem with tests only, let's keep this implementation only for tests and add suites handling later
| hermione.on(hermione.events.AFTER_TESTS_READ, (collection) => { | ||
| // cache all the tests to handle skipped tests in future | ||
| collection.eachTest((test) => { | ||
| loadedTests.set(test.fullTitle(), test); |
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.
test.fullTitle() is not unique value
this two tests have same fullTitle
describe("desc", function () {
it("it", () => {});
it("it", () => {});
});
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.
Done
| const hostname = os.hostname(); | ||
|
|
||
| const hermioneAllureReporter = (hermione: Hermione, opts: AllureReportOptions) => { | ||
| const loadedTests = new Map<string, Hermione.Test>(); |
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 don't really need Map in this case. Lets use Set maybe?
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.
Make id's uniq, but keep Map
vovsemenv
left a comment
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.
Lestgooo
Context
fixes #653
Checklist