Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
Support generative tests in @cleanup.#5809
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
anntzer commentedJan 7, 2016
Rebased#5785 onto this, just to show that this implementation works. |
tacaswell commentedJan 7, 2016
👍 that is way simpler than I expected it to be! |
anntzer commentedJan 7, 2016
Hum... I am pretty sure that the |
tacaswell commentedJan 7, 2016
Huh, good catch. I will try to take a look at that tomorrow. |
mdboom commentedJan 7, 2016
Thanks. We were just talking about this in#5718. |
tacaswell commentedJan 7, 2016
I do not have a clear memory of writing that test (but git blame tells me I did!). Sometimes I work in a tdd style where I just run the tests to check things so that test may have never worked. For expediency of getting this merged, maybe throw a KnownFail on this create an issue telling me to fix the test. |
anntzer commentedJan 7, 2016
Done. By the way, why was theprevious AppVeyor build successful even though the test suite failed? (I am a bit confused.) |
anntzer commentedJan 7, 2016
I am even more confused by the new test failure. Anyone wants to take over this PR? (TBH I have little interest in investigating this.) |
tacaswell commentedJan 8, 2016
Did you run these locally? That error is what happens if the test module is not importable. |
tacaswell commentedJan 8, 2016
Turns out the fix is trivial, there is just an extra |
tacaswell commentedJan 8, 2016
I am going to do some local git merging to:
|
tacaswell commentedJan 8, 2016
Actually, I am having issues getting this to pass locally and am getting a large number of file not closed warnings, seeanntzer#1 |
One too many assertionsCloses#1
anntzer commentedJan 8, 2016
Actually my original patch was wrong... can you see why? It's fixed now. |
Support generative tests in@cleanup.
Support generative tests in@cleanup.
tacaswell commentedMar 26, 2016
backported to v2.x as80be853 |
Support generative tests in@cleanup.
All's in the title.