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

Tests fail depending on which checksums are enabled #756

Open
daviddavis opened this issue Apr 28, 2023 · 3 comments
Open

Tests fail depending on which checksums are enabled #756

daviddavis opened this issue Apr 28, 2023 · 3 comments
Labels
.misc CHANGES/<issue_number>.misc Tests

Comments

@daviddavis
Copy link
Contributor

If you don't configure ALLOWED_CONTENT_CHECKSUMS, it defaults to everything but md5 and sha1. And it looks like tests will fail in this case:

E           pulpcore.exceptions.validation.UnsupportedDigestValidationError: Checksum algorithms ['md5', 'sha1'] are forbidden for this Pulp instance.

../pulpcore/pulpcore/app/models/content.py:269: UnsupportedDigestValidationError
============================================================== short test summary info ===============================================================
FAILED pulp_deb/tests/unit/test_models.py::TestPackage::test_filename - pulpcore.exceptions.validation.UnsupportedDigestValidationError: Checksum a...

I think ideally the test should either not rely on these checksums or be skipped if the checksums they require a certain checksum that is not enabled. Here's an example of the latter:

https://github.com/pulp/pulp_rpm/blob/77b19890daf5ceb7a11aa701033d81e97983468d/pulp_rpm/tests/functional/api/test_sync.py#L797-L800

@quba42
Copy link
Collaborator

quba42 commented May 3, 2023

This is by design/deliberate (which does not mean it should not be changed). Our reasoning was that we have configured the CI and our dev boxes to allow these checksums to facilitate these tests, so there was never any pressure to change. If you know of a better way to facilitate testing on boxes that don't have the setting, I am open to changing things.

@daviddavis
Copy link
Contributor Author

Did you catch my proposal to skip the test if the checksum is not enabled? Here's an example from pulp_rpm:

https://github.com/pulp/pulp_rpm/blob/77b19890daf5ceb7a11aa701033d81e97983468d/pulp_rpm/tests/functional/api/test_sync.py#L797-L800

@quba42
Copy link
Collaborator

quba42 commented May 3, 2023

@daviddavis I read the proposal to skip but I did not look at the RPM example. (One can see a lot by looking)

I am all for copying pulp_rpm and keeping plugins similar. Will remove the Triage-Needed label.

@quba42 quba42 added [noissue] For PRs with [noissue] in the commit message. (Change won't appear in the changelog). .misc CHANGES/<issue_number>.misc and removed Triage-Needed [noissue] For PRs with [noissue] in the commit message. (Change won't appear in the changelog). labels May 3, 2023
@quba42 quba42 added the Tests label Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.misc CHANGES/<issue_number>.misc Tests
Projects
None yet
Development

No branches or pull requests

2 participants