-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
TST: Add missing suffix to temppath manager #7149
Conversation
Without the suffix, np.save creates a new file and the file does not get cleaned up.
Actually, awesome as it is, the test always failed, seems nobody ever runs slow tests :) (this is the reason why adding slow tests to the other PR probably failed). |
OK, probably not true, it might have gotten a temppath fix, which is nice, but snuck in the error, so maybe it is recent. @charris we probably have to backport it, otherwise the slow tests will fail always. |
running the slow tests should be part of the release checklist fwiw I used to always run slow tests until this 4gb test was added, it always fails for me as my system always runs on ~1gb of free disk space ._. |
Travis seems happy to run it, as surprising as it is, so we can do that (I got that in my other PR, though could split that part of again). Yes, maybe it should have another category, or just skip if it fails with a disk space error (though in principle that could hide fails too). |
I don't think that test is too valuable for the time it takes. It is enough to run that once every few month or on PR's that touch related code. |
ASV takes much longer, running the slow tests on a single job does not actually take much longer I think, and I want to add a relatively slow test to check whether people use "ignore" filters or warnings.warn without a stacklevel, which would be nice to have a result at once. |
TST: Add missing suffix to temppath manager
The slow tests only take a couple of minutes, and there are quite important ones (like all or almost all
+1 I also run the full tests regularly, and that memory test doesn't really fit in
agreed with this as well. |
The xslow stuff sounds like a good idea, or high resource, or whatever, but frankly, I don't think I will put it on my TODO list ;). Btw. running the slow test on Travis seems to take maybe 3 minutes longer (will be a bit more with my test), if we only do it for a single test run, that is about 10% tops for the moment. |
Makes sense. Let's enable the slow tests for a single run asap, and then whoever gets annoyed enough at that high-memory test failing (good chance that that'll be me) can kick that out of |
Without the suffix, np.save creates a new file and the file
does not get cleaned up.
Trivial fix really.