Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Conversation

@ryanslade
Copy link
Contributor

@ryanslade ryanslade commented Nov 17, 2022

The previous tests we not testing cloning viap4-fusion, the following was happening:

git p4 test runs, completes and deletes external service which then soft
deletes repo. p4-fusion can't find repo to delete since it's name has changed
so does nothing. Repo is still on disk so we end up fetching rather than
cloning. Even if that fails, we still find the repo on disk and mark the test
as passed.

To fix this we now:

  • Delete the repo on disk before deleting the service. (Confirmed locally that the repo is removed on disk)
  • Clone a different repo in each test

The tests were also refactored to make things a little more explicit.

Test plan

Tested many times locally, confirm that passed integration tests in CI.

Also manually changes code locally so that the p4-fusion command would fail
to confirm we were actually running it.

Part of https://github.com/sourcegraph/sourcegraph/pull/44301

@cla-bot cla-bot bot added the cla-signed label Nov 17, 2022
@ryanslade ryanslade changed the title Temporary test to confirm that our integration tests fail if we run a non-existsent command instead of p4-fusion Temporary test to confirm that our integration tests fail if we run a non-existent command instead of p4-fusion Nov 17, 2022
@ryanslade ryanslade force-pushed the rs/test-fusion-failure branch from 2209940 to 75ff1bf Compare November 17, 2022 13:55
@ryanslade ryanslade changed the title Temporary test to confirm that our integration tests fail if we run a non-existent command instead of p4-fusion gqltest: Ensure fusion test is actually cloning depot Nov 17, 2022
@ryanslade
Copy link
Contributor Author

ryanslade commented Nov 17, 2022

Integration tests running here:
https://buildkite.com/sourcegraph/sourcegraph/builds/184120

@ryanslade ryanslade requested a review from a team November 17, 2022 14:27
@ryanslade ryanslade marked this pull request as ready for review November 17, 2022 14:27
@sourcegraph-bot
Copy link
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff cc7bd73...75ff1bf.

Notify File(s)
@unknwon dev/gqltest/external_service_test.go
dev/gqltest/sub_repo_permissions_test.go

@ryanslade
Copy link
Contributor Author

Once this is merged we should merge main back into https://github.com/sourcegraph/sourcegraph/pull/44301 and confirm that the new version can also clone a depot.

@ryanslade
Copy link
Contributor Author

@mrnugget PTAL

@burmudar
Copy link
Contributor

@cla-bot check

@cla-bot
Copy link

cla-bot bot commented Nov 17, 2022

The GitHub CLA Bot is rechecking to see that you have signed the CLA - note that it may take up to 30 minutes for your response to be synchronized.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants