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

Artifact Integration Tests Not Properly Isolated #30296

Closed
kdumontnu opened this issue Apr 5, 2024 · 1 comment · Fixed by #30300
Closed

Artifact Integration Tests Not Properly Isolated #30296

kdumontnu opened this issue Apr 5, 2024 · 1 comment · Fixed by #30300
Labels

Comments

@kdumontnu
Copy link
Contributor

Description

I've been having a lot of issues with failing integration tests due to the way artifact tests are implemented. It appears that the tests are not properly isolated from one-another. That is, tests depend on other tests running (successfully), and running multiple integration tests on the same file-system clobber each other.

For instance:

Fails, because it depends on TestActionsArtifactDownloadUploadSingle running

make clean build
make test-sqlite#TestActionsArtifactDownload

Passes, because it has a clean environment

make clean build
make test-sqlite#TestActions

(without cleaning) Fails, because there is data persisted.

make test-sqlite#TestActions

Integration tests shouldn't depend on other integration tests to run. Further, they should be cleaned up between each run.

Instead, we should initialize fixtures & data so that the download tests pass independently, and the upload tests use different resource names. This makes the errors more reproduce-able and easier to debug. For instance if a new code change is introduced in both upload and download routines, the tests may pass, but all existing deployments that exist can no longer download, because download wasn't tested independently.

I've started looking into this, but it's not clear initially how the data is being persisted (eg. file-system, db, queue, or some combination).

Gitea Version

release/v1.21

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

Locally, sqlite, go 1.21

Database

None

@KN4CK3R
Copy link
Member

KN4CK3R commented Apr 5, 2024

Don't know if my package tests are better but for example you can run TestPackageNuGet but not the sub tests TestPackageNuGet/Upload/DependencyPackage. The sub tests are just used as a form of documentation / segmentation.
If that's acceptable, the artifact tests could use a similar structure.

@lunny lunny closed this as completed in 66971e5 Nov 1, 2024
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this issue Nov 6, 2024
Closes go-gitea/gitea#30296

- Adds a DB fixture for actions artifacts
- Adds artifacts test files
- Clears artifacts test files between each run
- Note: I initially initialized the artifacts only for artifacts tests,
but because the files are small it only takes ~8ms, so I changed it to
always run in test setup for simplicity
- Fix some otherwise flaky tests by making them not depend on previous
tests

(cherry picked from commit 66971e591e5dddd5b6dc1572ac48f4e4ab29b8e0)

Conflicts:
	- tests/integration/api_actions_artifact_test.go
	  Conflict resolved by manually changing the tested artifact
	  name from "artifact" to "artifact-download"
	- tests/integration/api_actions_artifact_v4_test.go
	  Conflict resolved by manually updating the tested artifact
	  names, and adjusting the test case only present in our tree.
	- tests/test_utils.go
	  Resolved by manually copying the added function.
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jan 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants