Make all tests work offline

[cottsay]

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	resolves #59
Primary OS tested on	Fedora
Is this a breaking change?	No
Does this PR contain AI generated software?	No

---

Description of contribution in a few bullet points

Relying only on local resources results in more reliable test results and enables development without reliable internet access.

This change creates repository artifacts on-the-fly during test execution so that the same functionality can be validated without relying on live repositories on the internet.

Description of how this change was tested

• Performed linting validation using pre-commit run --all
• Verified that the code passes all tests using pytest -s -v test

[cottsay]

Okay, this appears to be working now, so I'm pulling it out of draft.

More context:
For the most part, I did my best to port the tests exactly as they are today and refrained from making any improvements or changes that didn't directly support the goal of running the tests offline. I did need to move the subversion and mercurial tests into a new class, but they probably shouldn't have been in the git-initialized class to begin with.

Happy to answer questions or discuss concerns.

P.S. - sorry about iterating on changes in the PR like this, I know there was a lot of force-pushing. I'll try to do better in the future.

[leander-dsouza]

The approach that you have taken is pretty impressive.

Building on the work, I feel that specifying all the repository files within the script gives little control to the user.

I am suggesting that we can use the following approach:

• Except for all the tar/zip and svn/bzr/hg vcs clients, we can simply mention the temporary paths to the repositories, while giving control to the user for tagging, commit hashes, and specific branch support:

  # Repo URL is fixed to local path for testing purposes
  ---
  repositories:
    immutable/hash:
      type: git
      url: file:///vcstmp/vcs2l
      version: 377d5b3d03c212f015cc832fdb368f4534d0d583
    immutable/tag:
      type: git
      url: file:///vcstmp/vcs2l
      version: tags/0.1.27
    vcs2l:
      type: git
      url: file:///vcstmp/vcs2l
      version: heads/main
    without_version:
      type: git
      url: file:///vcstmp/vcs2l

  By specifying the local paths, we do not have to change anything substantial in the testing scripts.

  Hence, manually tagging, checking out, and adding specific commits is not required, as this is supported natively when specifying paths with file:///.

• The helper script to set up the repository in a temporary directory can be split according to the VCS clients, for instance, test_helper_git, test_helper_tar, test_helper_svn, etc.

  Once the helper scripts are called before the actual testing, we can include the files with direct directories to the temporary named paths:

  list.repos

  # Repo URL is fixed to local path for testing purposes
  ---
  repositories:
    immutable/hash:
      type: git
      url: file:///vcstmp/vcs2l
      version: 377d5b3d03c212f015cc832fdb368f4534d0d583
    immutable/hash_tar:
      type: tar
      url: file:///vcstmp//vcs2l/archive/377d5b3d03c212f015cc832fdb368f4534d0d583.tar.gz
   version: vcs2l-377d5b3d03c212f015cc832fdb368f4534d0d583
    immutable/hash_zip:
      type: zip
      url: file:///vcstmp//vcs2l/archive/377d5b3d03c212f015cc832fdb368f4534d0d583.zip
   version: vcs2l-377d5b3d03c212f015cc832fdb368f4534d0d583
    immutable/tag:
      type: git
      url: file:///vcstmp/vcs2l
      version: tags/0.1.27
    vcs2l:
      type: git
      url: file:///vcstmp/vcs2l
      version: heads/main
    without_version:
      type: git
      url: file:///vcstmp/vcs2l

  list2.repos

  # Repo url is fixed to local path for testing purposes
  ---
  repositories:
    hg/branch:
      type: hg
      url: file:///vcstmp/hgrepo
      version: stable
    hg/hash:
      type: hg
      url: file:///vcstmp/hgrepo
      version: 9bd654917508
    hg/tag:
      type: hg
      url: file:///vcstmp/hgrepo
      version: 5.8
    svn/rev:
      type: svn
      url: file:///vcstmp/svnrepo
      version: 1

• These suggestions are mainly motivated by the incoming extends: field in #39, supporting multi-level inheritance. If the test files are separated from the scripts, then it is easy for the user to add checks for subsequent features to come.

[leander-dsouza]

P.S. - sorry about iterating on changes in the PR like this, I know there was a lot of force-pushing. I'll try to do better in the future.

No worries, Scott. Grateful for your contribution as always :)

[cottsay]

Hi, thanks for having a look.

I feel that specifying all the repository files within the script gives little control to the user.

On the contrary, there is now a list of commands for setting up the repository that the user can change or append to for any features they need tested. #74 already demonstrates this - we don't need to push any annotated tags to the vcs2l repository for the sole purpose of testing annotated tag support, we just need to update the staging repository creation commands to exercise the feature.

...we can simply mention the temporary paths to the repositories, while giving control to the user for tagging, commit hashes, and specific branch support

The main reason I switched to curating the repositories-under-test is that relying on the vcs2l repo itself will require use of git for fetching our source code for the tests to execute. Neither deb nor RPM packages use upstream development repositories for storing the project source code - both use an archive of some kind, usually a tarball. The tests are also typically included in a Python sourcedist, which is also an archive. Though cloning the git repository is necessary for users to develop changes to vcs2l that they wish to submit upstream, the tests are also useful as a PASS/FAIL indication of whether the package works as expected, especially with different versions of dependencies (Python, git, etc) on different platforms.

Once the helper scripts are called before the actual testing, we can include the files with direct directories to the temporary named paths

The file:///vcstmp/ path is only a placeholder that is replaced in the console output. The path doesn't actually exist and isn't used, we just couldn't let the captured console output contain the randomly generated temporary directory so it's substituted with that placeholder. The actual .repos files that are written to disk and operated on contain full paths.

The helper script to set up the repository in a temporary directory can be split according to the VCS clients

This is a reasonable request, and I think we should consider refactoring the tests to do so. It would also be nice if it was possible to run at least some of the tests without git, so splitting the archive tests out would be desirable.

As I mentioned in my initial comment, I'm really trying to focus this change on getting things offline and resist the urge to refactor the tests. This PR touches a lot of code and has a high likelihood to conflict with other changes so I'd like to get it reviewed and merged quickly, and we can iterate on more focused refactoring of the tests in follow-up changes.

Another refactor I'd like to do later is to leverage an additional temporary directory for the target of the import operation. We shouldn't just assume that the working directory is a good place to stash changes, and the tests can leave the temporary files on disk if you Ctrl-C in the right spot today, which makes subsequent test invocations fail persistently. Again, I'd like to keep refactors like this in separate PRs.

If the test files are separated from the scripts, then it is easy for the user to add checks for subsequent features to come.

That's the idea, really. The "setup" portion all happens in __init__.py and the fixtures can be used any number of times by other tests, though right now each fixture is only used in a single test class because that's how it was structured before. It's totally reasonable to add additional commits/branches/tags/etc to the staged repositories created by the fixtures or create additional fixtures which wrap the ones defined here in order to test additional features.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 26.98%. Comparing base (75727e7) to head (79c194d).

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #72   +/-   ##
=======================================
  Coverage   26.98%   26.98%           
=======================================
  Files          31       31           
  Lines        2238     2238           
  Branches      392      392           
=======================================
  Hits          604      604           
  Misses       1574     1574           
  Partials       60       60           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[leander-dsouza]

The file:///vcstmp/ path is only a placeholder that is replaced in the console output. The path doesn't actually exist and isn't used, we just couldn't let the captured console output contain the randomly generated temporary directory so it's substituted with that placeholder. The actual .repos files that are written to disk and operated on contain full paths.

Thank you for the explanation, Scott. I understand the use of named temporary directories in this case, my reasoning was that we could have a 'named' directory for each VCS client, so that the user can easily edit a future repos.yaml file to come.

As I mentioned in my initial comment, I'm really trying to focus this change on getting things offline and resist the urge to refactor the tests. This PR touches a lot of code and has a high likelihood to conflict with other changes so I'd like to get it reviewed and merged quickly, and we can iterate on more focused refactoring of the tests in follow-up changes.

Okay, sure, I understand that is where the priority lies.
Probably after this merge, future work would include the following:

• Separate helper scripts for each VCS client to help set up each temporary directory.
• Clearing temporary directories automatically upon cancellation/KeyboardInterrupt of the tests.
• Exposing integration with a repos.yaml file for seamless testing.

My immediate concern is that integration with #39 - multi-level inheritance will not be straightforward.
Probably once the future work is done, we can safely integrate #39 with main.

[nuclearsandwich]

As I mentioned in my initial comment, I'm really trying to focus this change on getting things offline and resist the urge to refactor the tests. This PR touches a lot of code and has a high likelihood to conflict with other changes so I'd like to get it reviewed and merged quickly, and we can iterate on more focused refactoring of the tests in follow-up changes.

+1 ticketing the test refactor and making an incremental improvement (that unblocks upstreaming into EPEL) is the approach that I would take here. Perfect being the enemy of good and all that.

[cottsay]

I responded to the PR feedback.

I don't think any of us are super thrilled with this PR. I have an idea, but before I do the work, I want to discuss. Would it be better, at this stage, to break this into two PRs?

1. Convert all of the existing repos files to templates so that we can substitute the file:/// urls to point to the on-disk repository locations. I'd probably introduce a single test fixture that expands all of the test/*.repos.in templates.
2. Make the offline assets
   a) Add new fixtures for each "client" type (gitrepo, svnrepo, hgrepo, archive.zip, archive.tar.gz)
   b) Wire all the appropriate fixtures together to align with the current tests.

This puts the stage fixtures for each "client" type in a better place for refactoring later, and avoids the on-the-fly creation of the .repos files in favor of templates.

Thoughts?

[leander-dsouza]

Thoughts?

Yes, I agree that the points that you've discussed are a great way to make improvements on this PR.

1. Convert all of the existing repos files to templates so that we can substitute the file:/// urls to point to the on-disk repository locations. I'd probably introduce a single test fixture that expands all of the test/*.repos.in templates.

The test fixtures could be structured in a way to preserve the style of list1.repos and list2.repos, by pointing to the onboard volatile temporary-named locations as file templates.

For instance:

• list.repos - Repository file template

# Repo URL is fixed to local path for testing purposes
---
repositories:
  hash:
    type: git
    url: file:///vcstmp/vcs2l
    version: $random_hash$
   tar:
    type: tar
    url: file:///vcstmp//vcs2l/archive/$random_hash$.tar.gz
    version: vcs2l-$random_hash$
   zip:
    type: zip
    url: file:///vcstmp//vcs2l/archive/$random_hash$.zip
    version: vcs2l-$random_hash$

2. Make the offline assets
   a) Add new fixtures for each "client" type (gitrepo, svnrepo, hgrepo, archive.zip, archive.tar.gz)
   b) Wire all the appropriate fixtures together to align with the current tests.

To achieve this, we can have separate scripts for each client - git, svn, hg, zip, and tar. This would be responsible for creating the temporary directories with the corresponding archives/checked-out repository files.

Therefore, the next two PRs can be as follows:

• Separate helper scripts for each VCS client to help set up each temporary directory.
• Exposing integration with a repos.yaml file for seamless testing.

Please let me know if I interpreted the next steps correctly. In addition, I have approved this PR and will merge it as soon as the conflicts are resolved.

[claraberendsen]

This looks good to me, we should extract the discussion to it's own issue