Skip to content
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 fullName use case #3

Merged
merged 2 commits into from
Oct 6, 2017
Merged

Fix fullName use case #3

merged 2 commits into from
Oct 6, 2017

Conversation

Vanuan
Copy link
Contributor

@Vanuan Vanuan commented Oct 6, 2017

Fix for some concerns raised in #2

@Vanuan
Copy link
Contributor Author

Vanuan commented Oct 6, 2017

With this change and the following test case:

// test1.js
describe('test1', () => {
  it('is a duplicate test', () => {});
});

// test1-2.js
describe('test1', () => {
  it('is a duplicate test', () => {});
});

// test1-3.js
describe('test1', () => {
  it('is a unique test', () => {});
});

// test4.js
describe('test4', => {
  it('is a unique test', () => {})
})

// test5.js
describe('test5', () => {
  it('is a unique test', () => {})
})

I see only one duplicate test (which is a correct behavior but doesn't suit my use case)

Duplicate test names found:
File:  /__tests__/test/test1-2.js
Title:  test1 is a duplicate test

@Vanuan
Copy link
Contributor Author

Vanuan commented Oct 6, 2017

Well, actually, I thought it should show me both tests with a duplicate name, shouldn't it?

@palmerj3
Copy link
Owner

palmerj3 commented Oct 6, 2017

@Vanuan with a little modification it definitely could. I can take a look tomorrow morning and add that. But thank you SO MUCH for this PR and the conversation.

Sorry earlier for the confusion. I really do appreciate the contribution. Is this project something you'd want to collaborate on?

@palmerj3 palmerj3 merged commit 028d4e6 into palmerj3:master Oct 6, 2017
@palmerj3
Copy link
Owner

palmerj3 commented Oct 6, 2017

I published 1.1.0 with this

@Vanuan
Copy link
Contributor Author

Vanuan commented Oct 6, 2017

Well, I'm currently investigating flaky tests in my company's project.
One of the issues I found was copy pasted code (not changing test names when reusing code).
I'm also using jasmineReporters.JUnitXmlReporter to generate xml files which are parsed by Jenkins.
So when one test has failed the build became flaky: depending on whether the passing or the failing test was run last the test result was either fail or pass.

So I found two options:

  1. Generating a unique filename using modifyReportFileName callback. It deemed impossible until I found something that you've done here: Add testPath to jest-jasmine2 reporter callbacks jestjs/jest#4594
  2. Detecting unique filenames separately. This project doesn't help this use case so much as fullName here and fullName in modifyReportFileName are different for some reason. Probably jest groups fullName by a test filename so it must use the top-level describe for it.

@palmerj3
Copy link
Owner

palmerj3 commented Oct 6, 2017

Awesome! I work on very similar things at my company.

I wrote this for when we needed junit xml files for the same purpose. You might give it a shot. https://github.com/palmerj3/jest-junit

@palmerj3
Copy link
Owner

palmerj3 commented Oct 6, 2017

And if you'd like to collaborate on this project or another project (for a better solution) I'd love to do that.

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