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

Mark downloads without preparedIds as PREPARING not RESTORING #40 #43

Merged
merged 2 commits into from
Jan 6, 2025

Conversation

patrick-austin
Copy link

@patrick-austin patrick-austin commented Nov 22, 2024

  • Jobs without preparedIds to be marked RESTORING get marked PREPARING instead, so that they get assigned one (preparedId) by the usual status checks.

Closes #40

Response response = adminResource.setDownloadStatus(testDownload.getId(), facilityName, adminSessionId, DownloadStatus.RESTORING.toString());
assertEquals(200, response.getStatus());

response = adminResource.getDownloads(facilityName, adminSessionId, "1 = 1");

Choose a reason for hiding this comment

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

Can a value of null be passed for the queryOffset? I was confused at first as to what the "1 = 1" was for but found it in other tests with a comment, so if null doesn't work, then at least a comment as to what it's for would be useful here.

Copy link
Author

Choose a reason for hiding this comment

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

I based the test on the other existing tests which used "1 = 1", taking the comment on faith (but not copying it across to the new test). Having had a look though it seems like the code should handle null values - these tests still passed for me locally without it so I've removed it in all three places. Maybe it didn't used to work, the comments were written, then the null case handling was added but the tests were never updated? In any case hopefully they are all simpler now.

@patrick-austin patrick-austin merged commit 74286ee into 36_queuing Jan 6, 2025
1 check failed
@patrick-austin patrick-austin deleted the 40_admin_put_status branch January 6, 2025 10:48
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.

2 participants