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

Remove pytest-lazy-fixture due to incompatibility with pytest v8.0.0 #369

Merged
merged 14 commits into from
Feb 7, 2024

Conversation

willGraham01
Copy link
Collaborator

@willGraham01 willGraham01 commented Feb 1, 2024

Resolves #368 |

Turns out we were only using pytest-lazy-fixture for one test anyway.

Workaround is to pass the fixture name to pytest.param as a string, and then use the requests fixture (built in to pytest) to fetch the value of that fixture.

As such, pytest-lazy-fixture is no longer a dependency.

Also fixes:

  • We're not shipping scikit-image even though cellfinder depends on it?

@willGraham01 willGraham01 requested a review from a team February 1, 2024 15:07
@alessandrofelder
Copy link
Member

Looks like tests on Ubuntu for Python 3.10 reproducibly encounter a segmentation fault 🤔 would you be able to investigate further, @willGraham01 , please?

@willGraham01
Copy link
Collaborator Author

Looks like tests on Ubuntu for Python 3.10 reproducibly encounter a segmentation fault 🤔 would you be able to investigate further, @willGraham01 , please?

Will do this afternoon / Monday after DocSkills teardown admin 👍

pyproject.toml Outdated Show resolved Hide resolved
@willGraham01
Copy link
Collaborator Author

willGraham01 commented Feb 6, 2024

Tests seem to be passing now, but holding off merging this PR due to the presence of #372 and an investigation can take place.

But if people need CI to pass, consider merging or rebasing onto this branch for the time being.

willGraham01 and others added 2 commits February 6, 2024 16:07
* improve detection debug logs

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* remove too wordy debug logs

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@willGraham01
Copy link
Collaborator Author

Investigations over on #376 seem to imply that the garbage collection fix was not necessary (all tests seem to be passing without the need for the garbage collection cleanup statement).

I suspect there might be a faulty cache, so have deleted the cache for this branch and requested a re-run of the original CI that threw the seg-fault error: https://github.com/brainglobe/cellfinder/actions/runs/7743237973/job/21314877990

willGraham01 and others added 3 commits February 7, 2024 11:56
Reverting apparently un-neccessary changes due to conflation with a bad cache fetch. 

* Revert "try attempting garbage collector to run at end of test"

This reverts commit 2753abb.

* Revert "Actually raise the exception for not enough CPUs"

This reverts commit 1545b1b.

* Revert "[pre-commit.ci] auto fixes from pre-commit.com hooks"

This reverts commit bdd0d91.

* Revert "Remove fixtures and get pytest to check for cpu availability at runtime"

This reverts commit 125fa9d.

* Revert "[pre-commit.ci] auto fixes from pre-commit.com hooks"

This reverts commit dd0e519.

* Revert "Clarify problem fixture docstrings"

This reverts commit 67caf82.

* Add a timeout to CI for runs
@alessandrofelder
Copy link
Member

Is the 20 min timout limit still sensible here, @willGraham01 ?

@alessandrofelder
Copy link
Member

Once this is merged, we should run the CI again for #366 #367 and #370 (and possibly others). If the tests pass #366 and #370 can be merged (as they are already approved).

@willGraham01
Copy link
Collaborator Author

Is the 20 min timout limit still sensible here, @willGraham01 ?

Good catch, will bump this to 1 hour (should be more than enough time for the Windows tests to complete as they're the slowest).

@willGraham01 willGraham01 merged commit abf7002 into main Feb 7, 2024
14 checks passed
@willGraham01 willGraham01 deleted the fix-ci-failures branch February 7, 2024 13:31
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.

[BUG] CI breaks with pytest v8.0.0
3 participants