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

DM-13046: Add random component to getTempFilePath filename #45

Merged
merged 4 commits into from Dec 18, 2017

Conversation

timj
Copy link
Member

@timj timj commented Dec 18, 2017

This fixes getTempFilePath such that there is a little bit of randomness in the constructed name. This has two major benefits:

  • meas_base can have two test classes in the same file sharing the same test method names.
  • you can now have nested context managers with the same file suffix. This I think will make @danielsf very happy since it was clear that lots of sims tests would have benefited from this.

This prevents problems when the same test method names is used
in different test case classes in the same file. It also allows
you to use nested context managers.
@timj timj requested a review from PaulPrice December 18, 2017 16:45
Copy link
Contributor

@PaulPrice PaulPrice left a comment

Choose a reason for hiding this comment

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

Very nice!

class TestNameClash1(unittest.TestCase):
"""The TestNameClashN classes are used to check that getTempFilePath
does not use the same name across different test classes in the same
file even if they have the same test methods."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be a module-level docstring or comment, since it's about a group of classes?

Point out you're attempting to trigger a race condition?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Can do that.

@timj timj merged commit cdd0db8 into master Dec 18, 2017
@ktlim ktlim deleted the tickets/DM-13046 branch August 25, 2018 05:57
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.

None yet

2 participants