Skip to content

move some git integration test setup into the test #83251

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

Draft
wants to merge 6 commits into
base: devel
Choose a base branch
from

Conversation

s-hertel
Copy link
Contributor

SUMMARY

Add a simple, unauthenticated git server for git tests that can't easily use the local protocol and replace a few external Github dependencies.

This should make it easier to add new tests for submodules, needed by #80456, and any bugfix for #83146.

Related #82013

ISSUE TYPE
  • Test Pull Request

@ansibot ansibot added test This PR relates to tests. needs_triage Needs a first human triage before being processed. labels May 14, 2024
@s-hertel s-hertel force-pushed the handle-more-git-test-setup branch from 6fbf3c7 to 6771ba0 Compare May 15, 2024 22:27
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label May 15, 2024
@s-hertel s-hertel force-pushed the handle-more-git-test-setup branch 2 times, most recently from 09f75e1 to de6c2ea Compare May 15, 2024 22:48
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label May 15, 2024
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label May 16, 2024
@s-hertel s-hertel force-pushed the handle-more-git-test-setup branch from 0fba50c to 23dd743 Compare May 28, 2024 17:46
@webknjaz

This comment was marked as resolved.

This comment was marked as resolved.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jun 12, 2024
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Apr 8, 2025
@s-hertel s-hertel force-pushed the handle-more-git-test-setup branch from 23dd743 to fc0876e Compare April 14, 2025 20:39
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Apr 14, 2025
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Apr 15, 2025
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Apr 15, 2025
@webknjaz
Copy link
Member

that can't easily use the local protocol

Could you expand on what this means? Is there a case where pointing a git remote to an fs-local .git folder path doesn't work?

- test_submodules_newer.git
exec:
- 'git submodule add --name submodule1 git://localhost/test_submodules_subm1.git submodule1'
# workaround for changing default submodule branch name
Copy link
Member

Choose a reason for hiding this comment

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

Can this be done through a .gitmodules file instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This does create .gitmodules.

repositories:
- test_submodules_newer.git
exec:
- 'git submodule add --name submodule1 git://localhost/test_submodules_subm1.git submodule1'
Copy link
Member

Choose a reason for hiding this comment

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

Usually, doing the following works just fine:

-          - 'git submodule add --name submodule1 git://localhost/test_submodules_subm1.git submodule1'
+          - 'git submodule add --name submodule1 file://path/to/bare/repo/test_submodules_subm1.git submodule1'

Or maybe

-          - 'git submodule add --name submodule1 git://localhost/test_submodules_subm1.git submodule1'
+          - 'git submodule add --name submodule1 ../relative/path/to/bare/repo/test_submodules_subm1.git submodule1'

(I don't remember which one works better, but I did use this in the past)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal of this PR is to replace dependencies on external Github repos that aren't managed by Ansible, and not be limited to 1 protocol since some features cannot be tested with file://.

The current tests use absolute URLs, so that's why these are absolute. I could use file:// for these specifically, but a git server is necessary to fully test relative submodules (which can be relative to the repo or origin remote), and because I know that submodules relative to the remote are broken (see #83146), I added the git daemon in this PR to make sure it works with our existing test cases, before I add more complex submodule tests.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. Thanks for explaining!

@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Apr 16, 2025
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Apr 18, 2025
@s-hertel s-hertel added the ci_verified Changes made in this PR are causing tests to fail. label Apr 18, 2025
@s-hertel
Copy link
Contributor Author

s-hertel commented Apr 18, 2025

I need to fix this change so the same error can occur for every retry attempt (it's difficult to spot the real cause of the test failure - https://dev.azure.com/ansible/ansible/_build/results?buildId=142962&view=logs&j=5e252db1-1db5-5540-3a64-4369eb0bd6ec&t=f64c9d35-7d74-5543-3054-69a140aa75b8&l=2377). #85015 fixes the cause of the test failure and fixes resetting between retry attempts.

@s-hertel s-hertel removed the ci_verified Changes made in this PR are causing tests to fail. label Apr 18, 2025
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Apr 23, 2025
@webknjaz
Copy link
Member

I'm re-adding ci_verified since you've confirmed earlier that this PR causes the problem, even if indirectly. So I think it should remain excluded from the CI maintenance queue.

@s-hertel s-hertel added unimportant_ci This PR does not need to have healthy CI status and should be ignored by the CI infra maintainers. and removed ci_verified Changes made in this PR are causing tests to fail. labels Apr 24, 2025
@s-hertel
Copy link
Contributor Author

Makes sense. Since this is just a POC anyway, I moved it out of the CI workflow for now.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. test This PR relates to tests. unimportant_ci This PR does not need to have healthy CI status and should be ignored by the CI infra maintainers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants