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

github: unit-tests: split unit tests to code specific workflows #15065

Merged
merged 2 commits into from
Feb 17, 2025

Conversation

bboozzoo
Copy link
Contributor

Split unit-tests to Go and C specific workflows.

Thanks for helping us make a better snapd!
Have you signed the license agreement and read the contribution guide?

Copy link

github-actions bot commented Feb 12, 2025

Fri Feb 14 16:07:13 UTC 2025

Spread tests skipped

@bboozzoo bboozzoo force-pushed the bboozzoo/unit-tests-split branch 2 times, most recently from 37a30aa to 76d38f7 Compare February 12, 2025 13:00
@bboozzoo bboozzoo marked this pull request as ready for review February 12, 2025 13:10
Copy link

codecov bot commented Feb 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.06%. Comparing base (a272aac) to head (7acc424).
Report is 14 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #15065      +/-   ##
==========================================
- Coverage   78.07%   78.06%   -0.01%     
==========================================
  Files        1182     1179       -3     
  Lines      157743   157702      -41     
==========================================
- Hits       123154   123114      -40     
+ Misses      26943    26940       -3     
- Partials     7646     7648       +2     
Flag Coverage Δ
unittests 78.06% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@bboozzoo bboozzoo added the Skip spread Indicate that spread job should not run label Feb 12, 2025
@bboozzoo bboozzoo closed this Feb 12, 2025
@bboozzoo bboozzoo reopened this Feb 12, 2025
- { code: go, go-build-tags: faultinject, skip-coverage: false, snapd-debug: false, go-test-race: false}
- { code: go, go-build-tags: statelocktrace, skip-coverage: true, snapd-debug: false, go-test-race: false}
- { code: go, go-build-tags: snapdusergo, skip-coverage: false, snapd-debug: false, go-test-race: false}
- { code: go, go-build-tags: "", skip-coverage: true, snapd-debug: false, go-test-race: true }
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need to add the code: go to the matrix data. You already pass it in statically above in the "with" section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, thanks for catching that. It's a leftover from before I moved C unit tests to a separate job.

description: 'Code to test (c, go)'
required: true
type: string
default: ''
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know that default is relevant here since the code input is required

@bboozzoo bboozzoo force-pushed the bboozzoo/unit-tests-split branch from 76d38f7 to 4cc9720 Compare February 12, 2025 13:52
@bboozzoo bboozzoo requested a review from maykathm February 12, 2025 14:46
@bboozzoo bboozzoo added the Simple 😃 A small PR which can be reviewed quickly label Feb 12, 2025
matrix:
test-case:
- { code: c, c-compiler: gcc }
- { code: c, c-compiler: clang }
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need the code: c here either. You already set it statically in the "with" section

@bboozzoo bboozzoo force-pushed the bboozzoo/unit-tests-split branch from 4cc9720 to 4ada18a Compare February 12, 2025 14:50
@bboozzoo bboozzoo requested a review from maykathm February 12, 2025 14:53
description: 'Code to test (c, go)'
required: true
type: string
default: 'go'
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I'm mistaken, I believe that when you mark an input as required, the workflow will throw an error if that input isn't present. If the default value will never be used, why have it? I would get rid of the default: 'go' line just to not add in irrelevant code.

@bboozzoo bboozzoo force-pushed the bboozzoo/unit-tests-split branch 2 times, most recently from 6bd175f to b6d8b0e Compare February 13, 2025 10:58
@bboozzoo bboozzoo requested a review from ZeyadYasser February 13, 2025 10:58
@bboozzoo bboozzoo force-pushed the bboozzoo/unit-tests-split branch from b6d8b0e to 3e3a751 Compare February 14, 2025 07:06
Copy link
Collaborator

@ernestl ernestl left a comment

Choose a reason for hiding this comment

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

Thanks, a suggestion for shorter less repetitive name concats

.github/workflows/unit-tests.yaml Outdated Show resolved Hide resolved
Split unit-tests to Go and C specific workflows.

Signed-off-by: Maciej Borzecki <[email protected]>
@bboozzoo bboozzoo force-pushed the bboozzoo/unit-tests-split branch from a46c3d9 to 7acc424 Compare February 14, 2025 15:27
@bboozzoo bboozzoo requested a review from ernestl February 14, 2025 15:29
Copy link
Collaborator

@ernestl ernestl left a comment

Choose a reason for hiding this comment

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

Thanks!

@bboozzoo bboozzoo merged commit 88e02d5 into canonical:master Feb 17, 2025
76 of 77 checks passed
@bboozzoo bboozzoo deleted the bboozzoo/unit-tests-split branch February 17, 2025 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Simple 😃 A small PR which can be reviewed quickly Skip spread Indicate that spread job should not run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants