Skip to content

Conversation

@IrvingMg
Copy link
Contributor

Moves image pulling to TestMain so it runs before the test timeout starts.

Fixes #325

@netlify
Copy link

netlify bot commented Dec 19, 2025

Deploy Preview for urunc ready!

Name Link
🔨 Latest commit 13f8c8c
🔍 Latest deploy log https://app.netlify.com/projects/urunc/deploys/6952afe8acb2b80008c5ac7f
😎 Deploy Preview https://deploy-preview-358--urunc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@IrvingMg
Copy link
Contributor Author

76.6% Duplication on New Code

I think this is a "false positive". SonarQube is detecting the test case struct definitions as duplicated code because they share the same fields (containerTestArgs). However, each test case has different values. These test cases were extracted from e2e_test.go to test_cases.go.

@cmainas
Copy link
Contributor

cmainas commented Dec 19, 2025

Thank you a lot @IrvingMg for this PR.

76.6% Duplication on New Code

I think this is a "false positive". SonarQube is detecting the test case struct definitions as duplicated code because they share the same fields (containerTestArgs). However, each test case has different values. These test cases were extracted from e2e_test.go to test_cases.go.

Yeah, feel free to ignore SonarQube. Just make sure that the tests run correctly. If there is any comment about the code, we will let you know.

Edit: Looking a bit in the failed tests (I might be totally wrong, so ignore me in that case). I think we will need to use each tool's pull method to pull the images, because ctr and crictl have different cli options for image pulling than nerdctl and docker.

@IrvingMg
Copy link
Contributor Author

I've looked into the failed tests, and one issue was caused by the cspell linter, while the second was related to the new filterTestCases function I introduced to filter which images to pull. Both issues should now be fixed.

Copy link
Contributor

@cmainas cmainas left a comment

Choose a reason for hiding this comment

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

Hello @IrvingMg ,

I do not have any comment for the code, It looks very good. However, I think that the filterTestCases function needs a small change. Currently, we are able to run specific test cases for a specific unikernel / monitor / rootfs etc. (e.g. make test_ctr_mirage runs all ctr tests for the MirageOS unikernel). With these changes, we are only able to match only based on the prefix of the subtest and hence make test_ctr_mirage will not work.

If this gets fixed, the PR is good to go.

@cmainas
Copy link
Contributor

cmainas commented Dec 29, 2025

Hello @IrvingMg , everything looks good. Just a small request to merge this without any CI failures. Would it be possible to add yourself in this file https://github.com/urunc-dev/urunc/blob/main/.github/contributors.yaml ? This is the file that our CI reads to check and add git trailers in the commits.

Optional: Also, if you want feel free to squash the last 3 commits.

@IrvingMg IrvingMg force-pushed the add-setup-step-e2e-test branch from 8fad81d to 632760d Compare December 29, 2025 13:41
@IrvingMg
Copy link
Contributor Author

@cmainas Thanks for reviewing! I’ve squashed the last three commits and added myself to the contributors file.

@cmainas
Copy link
Contributor

cmainas commented Dec 29, 2025

@cmainas Thanks for reviewing! I’ve squashed the last three commits and added myself to the contributors file.

Thank you @IrvingMg , it seems you did not sign off the last commit.

@IrvingMg IrvingMg force-pushed the add-setup-step-e2e-test branch from 632760d to 13f8c8c Compare December 29, 2025 16:44
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
76.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@IrvingMg
Copy link
Contributor Author

@cmainas Thanks for reviewing! I’ve squashed the last three commits and added myself to the contributors file.

Thank you @IrvingMg , it seems you did not sign off the last commit.

Oops, done :)

@urunc-bot urunc-bot bot changed the base branch from main to main-pr358 December 30, 2025 08:37
Copy link
Contributor

@cmainas cmainas left a comment

Choose a reason for hiding this comment

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

Thank you @IrvingMg

@cmainas cmainas merged commit fa0cc2d into urunc-dev:main-pr358 Dec 30, 2025
33 of 34 checks passed
github-actions bot pushed a commit that referenced this pull request Dec 30, 2025
…eout

PR: #358
Signed-off-by: IrvingMg <[email protected]>
Reviewed-by: Charalampos Mainas <[email protected]>
Approved-by: Charalampos Mainas <[email protected]>
github-actions bot pushed a commit that referenced this pull request Dec 30, 2025
PR: #358
Signed-off-by: IrvingMg <[email protected]>
Reviewed-by: Charalampos Mainas <[email protected]>
Approved-by: Charalampos Mainas <[email protected]>
github-actions bot pushed a commit that referenced this pull request Dec 30, 2025
PR: #358
Signed-off-by: IrvingMg <[email protected]>
Reviewed-by: Charalampos Mainas <[email protected]>
Approved-by: Charalampos Mainas <[email protected]>
github-actions bot pushed a commit that referenced this pull request Dec 30, 2025
PR: #358
Signed-off-by: IrvingMg <[email protected]>
Reviewed-by: Charalampos Mainas <[email protected]>
Approved-by: Charalampos Mainas <[email protected]>
urunc-bot bot pushed a commit that referenced this pull request Dec 30, 2025
…eout

PR: #358
Signed-off-by: IrvingMg <[email protected]>
Reviewed-by: Charalampos Mainas <[email protected]>
Approved-by: Charalampos Mainas <[email protected]>
urunc-bot bot pushed a commit that referenced this pull request Dec 30, 2025
PR: #358
Signed-off-by: IrvingMg <[email protected]>
Reviewed-by: Charalampos Mainas <[email protected]>
Approved-by: Charalampos Mainas <[email protected]>
urunc-bot bot pushed a commit that referenced this pull request Dec 30, 2025
PR: #358
Signed-off-by: IrvingMg <[email protected]>
Reviewed-by: Charalampos Mainas <[email protected]>
Approved-by: Charalampos Mainas <[email protected]>
urunc-bot bot pushed a commit that referenced this pull request Dec 30, 2025
PR: #358
Signed-off-by: IrvingMg <[email protected]>
Reviewed-by: Charalampos Mainas <[email protected]>
Approved-by: Charalampos Mainas <[email protected]>
@IrvingMg IrvingMg deleted the add-setup-step-e2e-test branch December 30, 2025 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create a setup step for each e2e subtest

2 participants