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 slow test marker #300

Merged
merged 1 commit into from
May 20, 2024
Merged

Remove slow test marker #300

merged 1 commit into from
May 20, 2024

Conversation

adamltyson
Copy link
Member

This PR closes #297 by removing the slow pytest marker.

I changed the test a bit, but it's still not ideal, because it changes the default brainglobe config file. It changes it back, but that's not great.

I assumed I could get around this by setting monkeypatch.setenv("BRAINGLOBE_CONFIG_DIR", str(tmp_path)), this however doesn't seem to set the config dir temporarily. I don't know if I'm monkeypatching this incorrectly, or there's a bug in the atlas api itself.

I've created this as a draft PR because I think this should be fixed, but maybe this PR should be merged and then an issue created to improve the test? This PR doesn't introduce the problem.

@adamltyson adamltyson marked this pull request as draft May 17, 2024 15:23
@alessandrofelder
Copy link
Member

I've created this as a draft PR because I think this should be fixed, but maybe this PR should be merged and then an issue created to improve the test? This PR doesn't introduce the problem.

I don't mind either way. A new issue is probably cleaner (but also more overhead). One thing maybe worth trying (that I think would be a good idea across BrainGlobe) is to monkeypatch Path.home() for all tests when not running on CI. It looks like there's a few constant variables that need to be adapted for the patching to work. This way

  • we test a slightly more real-life case on CI (Path.home() is not patched, but that's fine, because the CI does not have a user set up.)
  • we locally keep ~./brainglobe/ (user data) and ~/.brainglobe-tests/.brainglobe/ (tests interacting with fake user data) always completely separate.

@adamltyson
Copy link
Member Author

To simplify, I've marked this ready to review and raised #302

@alessandrofelder alessandrofelder merged commit 513ddf3 into main May 20, 2024
11 checks passed
@alessandrofelder alessandrofelder deleted the slow-tests branch May 20, 2024 09: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.

Ensure all tests are run
2 participants