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

Restrict files written by tests #1061

Merged
merged 22 commits into from
Sep 5, 2023
Merged

Restrict files written by tests #1061

merged 22 commits into from
Sep 5, 2023

Conversation

flying-sheep
Copy link
Member

@flying-sheep flying-sheep commented Jul 17, 2023

@flying-sheep flying-sheep changed the title config changes Stop tests from writing files everywhere Jul 17, 2023
@flying-sheep flying-sheep changed the title Stop tests from writing files everywhere forbid tests from writing files everywhere Jul 17, 2023
@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Merging #1061 (0be49dc) into main (fea7561) will decrease coverage by 0.15%.
The diff coverage is 37.50%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1061      +/-   ##
==========================================
- Coverage   84.90%   84.75%   -0.15%     
==========================================
  Files          36       36              
  Lines        5133     5149      +16     
==========================================
+ Hits         4358     4364       +6     
- Misses        775      785      +10     
Flag Coverage Δ
gpu-tests 52.41% <37.50%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
anndata/compat/__init__.py 77.43% <37.50%> (-3.57%) ⬇️

@flying-sheep flying-sheep requested a review from ivirshup July 17, 2023 13:32
@flying-sheep
Copy link
Member Author

@ivirshup implemented your suggestions from the call!

@flying-sheep flying-sheep self-assigned this Aug 7, 2023
@flying-sheep flying-sheep changed the title forbid tests from writing files everywhere Restrict files written by tests Aug 21, 2023
Copy link
Member

@ivirshup ivirshup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you give a summary of what goes where? Is it only coverage that's going into test-data?

pyproject.toml Outdated Show resolved Hide resolved
conftest.py Show resolved Hide resolved
.azure-pipelines.yml Show resolved Hide resolved
anndata/compat/__init__.py Show resolved Hide resolved
@ivirshup
Copy link
Member

ivirshup commented Sep 5, 2023

Also, I get why you want to make the data files not be written to the root. Why move test outputs like reports and coverage from their default locations?

@flying-sheep
Copy link
Member Author

flying-sheep commented Sep 5, 2023

Can you give a summary of what goes where? Is it only coverage that's going into test-data?

documented in .gitignore:

anndata/.gitignore

Lines 17 to 18 in 4c016d2

# Test results and coverage
/test-data/

Why move test outputs like reports and coverage from their default locations?

so test outputs end up in one location that can be .gitignored instead of polluting the project directory.

Copy link
Member

@ivirshup ivirshup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to change local behavior, so have made a set of suggestions reverting that. Otherwise looks good.

documented in .gitignore:

I'm not completley clear on what counts as a "test-result".

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
flying-sheep

This comment was marked as resolved.

Co-authored-by: Isaac Virshup <[email protected]>
@flying-sheep
Copy link
Member Author

flying-sheep commented Sep 5, 2023

I'm not completley clear on what counts as a "test-result".

plural. a file recording the test results. in the “nunit” or “junit” format

Otherwise looks good.

then I’ll merge.

.gitignore Outdated Show resolved Hide resolved
Co-authored-by: Isaac Virshup <[email protected]>
@flying-sheep flying-sheep enabled auto-merge (squash) September 5, 2023 15:57
@flying-sheep flying-sheep merged commit 2c8759d into main Sep 5, 2023
12 checks passed
@flying-sheep flying-sheep deleted the fix-pollution branch September 5, 2023 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stop tests from writing files everywhere
2 participants