Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2288 +/- ##
==========================================
+ Coverage 54.49% 54.69% +0.19%
==========================================
Files 409 411 +2
Lines 43970 44305 +335
Branches 8100 8175 +75
==========================================
+ Hits 23961 24232 +271
- Misses 19190 19246 +56
- Partials 819 827 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Hook25
left a comment
There was a problem hiding this comment.
Thanks for this. Please see the following:
| if grep -qrE 'private-ppa\.launchpad(|content)\.net' "/etc/apt/sources.list"* ; then | ||
| >&2 printf 'The following files have private PPA access:\n' | ||
| >&2 grep -lrE 'private-ppa\.launchpad(|content)\.net' "/etc/apt/sources.list"* | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
this is incomprehensible to all non-bash masters. Please either make it easier to read or rewire it in python.
I need to easily be able to tell:
- What are the matchers (I wouldn't use the regex here, split this into 2 checks, tbh just matching private-ppa should be enough)
- What happens if that file doesn't exist
- What is that this is supposed to print when it finds something funny
- This doesn't read sources.list.d or am I mistaken? If I am mistaken, please make this clear
There was a problem hiding this comment.
Also, minor concern but there is not much of a difference between writing to std out or stderr, why did you redirect to stderr? it makes the line slightly harder to read and afaik that syntax is a bashism
There was a problem hiding this comment.
Thank you for the review:)
- Now it is 2 checks. If I think of it again, I could've just checked for
private-ppainstead - prints "No private PPAs found in sources list files" and returns 0
- Prints the name of the file that has private-ppa in it and returns 1
- Reads both. Now I have added a an extra line to make it clear:)
| miscellanea/ubuntu-desktop-recommends certification-status=blocker | ||
| miscellanea/ubuntu-desktop-minimal-recommends certification-status=blocker | ||
| miscellanea/grub_file_check certification-status=blocker | ||
| miscellanea/screen-private-ppa |
There was a problem hiding this comment.
You can't add this test here, this testplan is ran everywhere and will cause a lot of issues to all test plans that nest this. Please add it to your own testplan only
This test check if any private PPAs are present in the system, fails if present. This is to avoid any leak of private PPA tokens in the release image.
6ff6c91 to
e05bdec
Compare
WARNING: This modifies com.canonical.certification::sru-server
Description
This test is to avoid any leak of private PPA credentials in the release image.
This checks if any private PPAs are present in the system, fails if present.
adds
miscellanea/screen-private-ppatest to baseThis is same as #2212, but I want to the test in base provider instead.
Resolved issues
Resolves: https://warthogs.atlassian.net/browse/PECA-1148
Documentation
Tests
Tested on Qualcomm RB3Gen2 Lite, should work on any system.