Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

Add test for recursive_conservative copy #156

Merged
merged 1 commit into from
Jan 31, 2019
Merged

Add test for recursive_conservative copy #156

merged 1 commit into from
Jan 31, 2019

Conversation

nixocio
Copy link
Contributor

@nixocio nixocio commented Jan 29, 2019

Add test for recursive_conservative copy. Explore the newly added flag
recursive_conservative used together with recursive.

See: https://pulp.plan.io/issues/4269

@nixocio
Copy link
Contributor Author

nixocio commented Jan 29, 2019

@dralley @goosemania, please, confirm that those test scenarios described are sane.

@nixocio nixocio requested a review from bherrin3 January 29, 2019 19:43
@nixocio nixocio changed the title Add test for recursive_conservative copy [WIP] Add test for recursive_conservative copy Jan 29, 2019
dralley
dralley approved these changes Jan 29, 2019
View reviewed changes
Copy link
Contributor

@dralley dralley left a comment

Choose a reason for hiding this comment

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

This looks right to me.

My only recommendation is that there be a test case for recursive=False and recursive_conservative=False, that it only copies the chimpanzee RPM and nothing else. But that might already be tested elsewhere, I just didn't see it when skimming around.

@nixocio
Copy link
Contributor Author

nixocio commented Jan 29, 2019 •
edited
Loading

@dralley, we do have tests for simple copy. But I will add this scenario that you described, since we can now use the recursive_conservative as part of the simple copy.

@nixocio nixocio force-pushed the expand_test_4269 branch 3 times, most recently from 471bcd4 to 498676d Compare January 31, 2019 14:37
@nixocio nixocio changed the title [WIP] Add test for recursive_conservative copy Add test for recursive_conservative copy Jan 31, 2019
Add test for recursive_conservative copy
1818b50 
Add test for recursive_conservative copy. Explore the newly added flag
`recursive_conservative` used together with `recursive`.

See: https://pulp.plan.io/issues/4269
@bherrin3
Copy link
Contributor

@kersommoura just a note here for myself that the testing I did against a local 2.18.1b1 FIPs installation had failures on CopyConservativeTestCase.test_norecursive_conservative_dependency.

Just ping me when that is resolved. Thanks!

@nixocio
Copy link
Contributor Author

nixocio commented Jan 31, 2019

@goosemania, I updated per your explanation.

Tested against 2.18.1b1 RHEL7.6.

@nixocio
Copy link
Contributor Author

nixocio commented Jan 31, 2019

@bherrin3, I just updated an assertion as per @goosemania explanation.

bherrin3
bherrin3 approved these changes Jan 31, 2019
View reviewed changes
Copy link
Contributor

@bherrin3 bherrin3 left a comment

Choose a reason for hiding this comment

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

Tested against RHEL7.6 2.18.1b1 FIPS Only.

Test loop, linting, and docs all look good.

@nixocio nixocio merged commit a26408c into pulp:master Jan 31, 2019
@nixocio nixocio mentioned this pull request Jan 31, 2019
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dralley dralley dralley approved these changes

@bherrin3 bherrin3 bherrin3 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nixocio @bherrin3 @dralley