-
Notifications
You must be signed in to change notification settings - Fork 117
[bugfix] Do not issue a name conflict error for the same fixture imported from different test files #2780
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
[bugfix] Do not issue a name conflict error for the same fixture imported from different test files #2780
Conversation
Codecov ReportBase: 86.61% // Head: 86.58% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2780 +/- ##
==========================================
- Coverage 86.61% 86.58% -0.04%
==========================================
Files 60 60
Lines 11218 11230 +12
==========================================
+ Hits 9717 9723 +6
- Misses 1501 1507 +6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
1b3e5b6 to
3abe4ea
Compare
ekouts
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.
Would it be easy to make the PR to master, since it is a bugfix? Otherwise, it looks good to me
vkarak
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.
I think you started off the branch from the develop and now trying to merge with master. You should branch off master and apply your changes there.
8728ecc to
e25f156
Compare
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.
I think that this fix introduces another bug. :-) The reason that the framework refuses to run tests with conflicting names is because they cannot execute safely in parallel, as they will step on each other in the stage directory. This fix basically allows conflicting tests defined in different files to run together, which is wrong. However, the problem in #2759 is that fixtures with the same name are disallowed. This is a bug, because fixtures with the same name (i.e., with the same scope and same variables defined) are guaranteed to run only once, so the framework should not complain about them being defined multiple times. Therefore the fix should be to ignore name conflicts in case of fixtures as fixtures are guaranteed to get a unique name and each one of those will be executed once per scope.
Signed-off-by: Theofilos Manitaras <manitaras@cscs.ch>
e25f156 to
fc74289
Compare
Signed-off-by: Theofilos Manitaras <manitaras@cscs.ch>
Signed-off-by: Theofilos Manitaras <manitaras@cscs.ch>
Fixes #2759