-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
tests: add default_root_dir=tmpdir #2392
Conversation
7fbfd0a
to
5ff2b08
Compare
Not sure why MLFlow fails in CI while locally pass
|
same here #2391 what's going on? I've been trying to debug this for literally almost the whole day. |
@awaelchli so yo say that |
yes, the bug is somewhere burried in the many if else statements that sets these two variables. There seems to be a mismatch betwen these two paths. |
at first I thought just to skip it and use the passing test maybe it is a chance to find the bug and fix it... |
This pull request is now in conflict... :( |
5ff2b08
to
1bada9d
Compare
This pull request is now in conflict... :( |
1bada9d
to
dd65024
Compare
Codecov Report
@@ Coverage Diff @@
## master #2392 +/- ##
======================================
Coverage 91% 91%
======================================
Files 82 82
Lines 6784 6784
======================================
Hits 6151 6151
Misses 633 633 |
@awaelchli thx for fixing missed changes after rebase :] |
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.
LGTM 👍
excited for this PR, those lingering test artifacts are definitely annoying
4f569ca
to
801e507
Compare
What does this PR do?
some test were influenced by previous runs...
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃