Skip to content

ENH: enable/disable resource monitor in the fixture per test #2725

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

Merged
merged 2 commits into from
Oct 11, 2018

Conversation

yarikoptic
Copy link
Member

This is just a more "kosher" way to avoid side-effects across tests etc.
It was "inspired" to mitigate an issue of some stale processes being
kept in background preventing pytest run completion, see
#2719

So, even though it avoids that issue now (both tests are skipped anyways
unconditionally ATM), it does not solve it -- some issue causing that
was introduced somewhere between 1.1.2 and 1.1.3

This is just a more "kosher" way to avoid side-effects across tests etc.
It was "inspired" to mitigate an issue of some stale processes being
kept in background preventing pytest run completion, see
nipy#2719

So, even though it avoids that issue now (both tests are skipped anyways
unconditionally ATM), it does not solve it -- some issue causing that
was introduced somewhere between 1.1.2 and 1.1.3
@effigies effigies requested a review from oesteban October 10, 2018 13:26
@yarikoptic
Copy link
Member Author

not sure what is up with those circleci fails - seems unrelated, waiting on travis. Locally debian builds seems finally started to pass that hiccup

@effigies
Copy link
Member

Waiting on #2707.

@effigies
Copy link
Member

@yarikoptic Can you merge master? That may fix tests.

* origin/master:
  FIX: Pin coverage<5
  [FIX] Pin codecov to <5.0 so Travis is fixed
@yarikoptic
Copy link
Member Author

done

@effigies
Copy link
Member

Hoo boy, it sure didn't.

@yarikoptic
Copy link
Member Author

well -- travis is green
What confuses me also that there is no codecov report and chromium plugin says that the test file is not covered. But otherwise it is "ready" afaik -- shouldn't make things worse and allows me to "pass" the tests

@effigies
Copy link
Member

Codecov considers any failing tests as potentially missing information, and avoids posting incomplete coverage summaries.

I'm okay with merging, as this is a necessary fix. We'll deal with the rest as we can.

@effigies effigies merged commit d1a9e96 into nipy:master Oct 11, 2018
@effigies effigies added this to the 1.1.4 milestone Oct 11, 2018
@yarikoptic yarikoptic deleted the bf-resource-monitor branch January 11, 2019 18:54
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.

3 participants