-
Notifications
You must be signed in to change notification settings - Fork 736
[CI] run tics on weekly schedule #3953
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3953 +/- ##
=======================================
Coverage 89.44% 89.44%
=======================================
Files 259 259
Lines 14758 14758
=======================================
Hits 13200 13200
Misses 1558 1558 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3decb72 to
75a7763
Compare
75a7763 to
69fff64
Compare
|
Hey Scott, it looks like there is an issue with tools folder: https://canonical.tiobe.com/tiobeweb/TICS/TqiDashboard.html#axes=Project(multipass),Sub()&metric=tqiTestCoverage&sel=Sub(tools) |
|
@ricab I think its out of my control. I can see the same "error" in the initial TICS run that was run by Tiobe staff. Comparing between the different time stamps, I've noticed some times slight changes between runs even though the source code was the same. I also Tiobe is actively developing the backend of TICS and analysis results may be subject to changes. The TICS github action used here is simply a client that calls some backend API endpoint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Scott, I have a few questions.
.github/workflows/tics.yml
Outdated
| sudo apt-get update | ||
| sudo apt-get install -y devscripts equivs lcov | ||
| mk-build-deps -s sudo -i | ||
| sudo --preserve-env apt-get install --yes devscripts equivs lcov ninja-build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please explain why --preserve-env was necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And why not update? Are these runner's apt cache guaranteed to be up to date?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how Mir had it set up so I copied it, but I just tested and it isn't necessary. The variables that --preserve-env is protecting isn't related to building Multipass anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, makes sense. How about the missing apt update?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have this b/c:
https://github.com/canonical/mir/blob/df6674bd3686286aa0e422fdc26664e1fbef5adf/.github/workflows/tics.yml#L24-L25
You might get hit with some dependency that requires configuration, and the install will seem stuck b/c it will wait on user input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No reason to exclude it so I added it back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, interesting. Thanks @Saviq, is this only a concern in these runners? I suppose GH could be addressing that under the hood.
@sharder996, might be a good idea to do the same then (i.e. avoid apt-get asking what services should be restarted.)
781b44e to
d82e81d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sharder996, this looks good to me now!
As discussed, we have a little margin to speed up with ccache if we need to in the future. Otherwise just needs a rebase.
We no longer check the network type when consuming networks and only look at the network name. In certain environments `eth0` is a valid network and causes this test to fail.
d82e81d to
ac69de0
Compare
Adds Tiobe's TICS github action to the Multipass project
${{ github.workspace }}/coverage/coverage.xml${{ github.workspace }}/buildmakemust be used for compilation (usingninjagenerator will fail the action)tests/subdirectory (this is inline with similar projects such as Mir)MULTI-1759
Closes #3962