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

Disregard old test repository quirks for migration #4954

Merged

Conversation

CasperWA
Copy link
Contributor

An old test created a .gitignore file in the Node repository folder
path, while also creating files under a raw_input folder.
This adds a logic test for this specific edge-case, migrating the
raw_input folder and ignoring the path folder for the specific Node.

Fixes #4910.

@CasperWA CasperWA requested review from ltalirz and sphuber May 19, 2021 11:09
@codecov
Copy link

codecov bot commented May 19, 2021

Codecov Report

Merging #4954 (b5ff789) into develop (b5b5051) will increase coverage by 0.01%.
The diff coverage is 66.67%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4954      +/-   ##
===========================================
+ Coverage    80.11%   80.11%   +0.01%     
===========================================
  Files          515      515              
  Lines        36665    36666       +1     
===========================================
+ Hits         29369    29371       +2     
+ Misses        7296     7295       -1     
Flag Coverage Δ
django 74.58% <66.67%> (+0.01%) ⬆️
sqlalchemy 73.50% <66.67%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
aiida/backends/general/migrations/utils.py 89.19% <66.67%> (+0.05%) ⬆️
aiida/transports/util.py 65.63% <0.00%> (+3.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5b5051...b5ff789. Read the comment docs.

@chrisjsewell
Copy link
Member

cheers @CasperWA, just to confirm there is not an analogous change required in the sqlalchemy migrations?

@chrisjsewell
Copy link
Member

well actually I guess I meant test for the sqlalchemy backend, since the actual change is in general utils, so I assume that applies to both

@CasperWA
Copy link
Contributor Author

CasperWA commented Jun 15, 2021

well actually I guess I meant test for the sqlalchemy backend, since the actual change is in general utils, so I assume that applies to both

Yeah, it seems the checks were done backend-agnostically here, which makes sense as it has nothing to do with the DB. But I'm unsure exactly what you mean otherwise here? Do you want a unittest for something or?

Edit: Ah, I think you do want another unit test, but for the SQLAlchemy backend. Hmm, I don't think it's necessary, but I can set it up if you want, just to be absolutely sure?

@chrisjsewell
Copy link
Member

Ah, I think you do want another unit test, but for the SQLAlchemy backend. Hmm, I don't think it's necessary, but I can set it up if you want, just to be absolutely sure?

Yeh that was the point; I just initially noted there was a django named file, but not an sqla one lol.
If its possible to do easily/quickly, I guess it would be nice? (although I'm sure we have better things to do with our lives)

@CasperWA
Copy link
Contributor Author

If its possible to do easily/quickly, I guess it would be nice? (although I'm sure we have better things to do with our lives)

Mnjeh - you'd say so wouldn't you? 😏 😅

Anyway, I've made a test, but now I'm not remembering how to test the SQLAlchemy backend, what's the env var?

@CasperWA
Copy link
Contributor Author

Anyway, I've made a test, but now I'm not remembering how to test the SQLAlchemy backend, what's the env var?

Nevermind, I figurereded it out. I'm totally sticky brain.

(But for real: AIIDA_TEST_BACKEND=sqlalchemy pytest...) 👍

CasperWA added 2 commits June 15, 2021 15:50
An old test created a `.gitignore` file in the Node repository folder
`path`, while also creating files under a `raw_input` folder.
This adds a logic test for this specific edge-case, migrating the
`raw_input` folder and ignoring the `path` folder for the specific Node.

Fixes aiidateam#4910.
@CasperWA CasperWA force-pushed the fix_4910_gitignore-in-raw_input-and-path branch from 317b05f to b5ff789 Compare June 15, 2021 13:50
@CasperWA CasperWA requested a review from chrisjsewell June 15, 2021 13:51
Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

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

Merge that bad boy!

@CasperWA CasperWA merged commit 28ef3a5 into aiidateam:develop Jun 15, 2021
@CasperWA CasperWA deleted the fix_4910_gitignore-in-raw_input-and-path branch June 15, 2021 14:17
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.

Object store migration: Node folders raw_input vs. path
2 participants