add integration tests for pip wheels

[slimreaper35]

testing repository:
https://github.com/cachito-testing/cachi2-pip-wheels

Maintainers will complete the following section

• Commit messages are descriptive enough
• Code coverage from testing does not decrease and new code is covered
• [n/a] Docs updated (if applicable)
• [n/a] Docs links in the code are still valid (if docs were updated)

Note: if the contribution is external (not from an organization member), the CI
pipeline will not run automatically. After verifying that the CI is safe to run:

• approve GitHub Actions workflows by clicking a button
• approve the Red Hat Trusted App Pipeline container build by commenting /ok-to-test
  (as is the standard for Pipelines as Code)

[slimreaper35]

I am not sure about e2e test. The build uses wheels from requirements.txt and requirements-build.txt. If the allow binary option is set to false, the build will still pass because of requirements-build.txt.

Should I remove requirements-build.txt or/and add a scenario where the build fails because of missing requirements-build.txt ? What do you think?

[eskultety]

I'm okay with the change with a small nitpick and a question on the e2e test.
Also, we should probably include an update to the existing references to the now-nonexistent https://github.com/cachito-testing/cachito-pip-without-metadata.git repository.

[eskultety]

I am not sure about e2e test. The build uses wheels from requirements.txt and requirements-build.txt. If the allow binary option is set to false, the build will still pass because of requirements-build.txt.

Should I remove requirements-build.txt or/and add a scenario where the build fails because of missing requirements-build.txt ? What do you think?

I think this is the wrong way to think about this. If we purposefully leave out build requirements then it's IMO not a valid e2e test. Instead, I think we need an e2e test that requires allow_binary=true because otherwise it would simply not build at all, so either you'll need to find build requirements that only provide wheels or write an app that depends on a package which only distributes as a wheel.

[slimreaper35]

I updated the test data so it reflects #508

[taylormadore]

Have we covered all of the test cases listed in the story description? Specifically the ones related to hashes and the one about testing a build that requires wheels?

about testing a build that requires wheels

I thought the e2e test might cover this, but it appears to pass with or without allow_binary

[slimreaper35]

Have we covered all of the test cases listed in the story description?

I think so, except hash checking is/isn't used. I just can't find a distribution package, that does not report hashes.
If there are no hashes reported by PyPI and no hashes in the requirements.txt => we should download the package without verifying the hash.

[ben-alkov]

Have we covered all of the test cases listed in the story description?

I think so, except hash checking is/isn't used. I just can't find a distribution package, that does not report hashes.

Yep, agreed. AFAUI, it's a baked-in part of the PyPi API - the only way you'd get a request reply without a checksum would be someone spoofing you (or maybe if you're not talking to PyPi but rather some other index, but even then I'm not certain).

If there are no hashes reported by PyPI and no hashes in the requirements.txt => we should download the package without verifying the hash

This should read "we should NOT download the package" - no hashes at all = no download.

[eskultety]

@slimreaper35 I don't see any update to the e2e tests really, so what is your stance on #504 (comment) as you haven't replied there really whether you agree or disagree with that argument and this PR then got stuck. From my POV in this case it makes sense having a single e2e test that would test both wheels and sdists at the same time, why? Because fetching is what we really only care about and since we've taken care of testing fetches in different scenarios now we just need to make sure that whatever we fetched can be built with.

[slimreaper35]

Update (finally):

• There is only one integration test with packages that have no wheels and allow_binary option is set to true
• There is only one e2e test with tensorboard package that have no sdists and again allow_binary option is set to true
  Luckily I found a package that does not generate a huge requirements.txt file... The pre-fetching with wheels should take only ~ 50 seconds
  Should I add test ensuring that pre-fetching will fail on allow_binary option set to false ?

[eskultety]

  Should I add test ensuring that pre-fetching will fail on `allow_binary` option set to false ?

Yes we should have such a test. In fact, you had two in earlier revisions, but for some reason you dropped it here: https://github.com/containerbuildsystem/cachi2/compare/ce01911eb42de201fd4d9507ad9d3a46114c23ba..06d216f8de883042e36bbbe47f29a0e452ccf5f8#diff-e4d828af95a3a935bef8644268aee7ff4190648a95b7a401e91a65dbebb9a206L99, any particular reason for that? IIRC I even emphasized the importance of those isolated fail cases here: #504 (comment)