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

tests: removing uc16 executions #14575

Merged

Conversation

sergiocazzolato
Copy link
Collaborator

This task removes tests executions for uc16 but the support remains because we need to test core snap for security releases.

Other changes are included to move some tests from uc16 to uc18.

A new nightly job will be included to run uc16 tests and nested tests.

Copy link

codecov bot commented Oct 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.02%. Comparing base (96ea7b0) to head (3d9599e).
Report is 76 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #14575      +/-   ##
==========================================
+ Coverage   78.95%   79.02%   +0.07%     
==========================================
  Files        1084     1087       +3     
  Lines      146638   147628     +990     
==========================================
+ Hits       115773   116670     +897     
- Misses      23667    23729      +62     
- Partials     7198     7229      +31     
Flag Coverage Δ
unittests 79.02% <ø> (+0.07%) ⬆️

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.

@@ -7,8 +7,6 @@ details: |
the new kernel snap. Finally check it is possible to remodel
back to the initial model.

# FIXME: add core18 test as well
# TODO:UC20: enable for UC20
systems: [ubuntu-core-16-64]
Copy link
Member

Choose a reason for hiding this comment

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

Why are we removing the TODO but not changing the systems?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is because we are testing different kernel remodeling in nested tests, and this one cannot be updated to work in uc20+ without a big refactor.

Copy link
Member

Choose a reason for hiding this comment

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

can we rename the test to specifically indicate this one is for uc16 and not generic. The test name should probably be uc16-remodel-kernel and not remodel-kernel

Copy link
Member

Choose a reason for hiding this comment

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

Otherwise it will give a false sense of security when looking at the test overview, we want to be explicit we are not testing this for other systems, and that we in fact need a new test for this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated

@@ -9,7 +9,7 @@ details: |
that, after a snap declared a iio plug is installed and connected, it can
access the node and read/write its content.

systems: [ubuntu-core-16-64]
systems: [ubuntu-core-*-64]
Copy link
Member

@Meulengracht Meulengracht Oct 9, 2024

Choose a reason for hiding this comment

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

This test seems to be failing on all the other UC systems - this was due to IIO missing in snapd right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right, I see this error:
error: snap "snapd" has no slot named "iio0"

@Meulengracht @alfonsosanchezbeato, What do you suggest? Should we remove the test? should we add that interface to snapd?

Copy link
Member

@Meulengracht Meulengracht Nov 7, 2024

Choose a reason for hiding this comment

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

If it was intentional the interface does not exist in the snapd snap, but exists on the core snap, then this must exist just as an UC16 test, and that means we should remove it from master. As long as this runs on the branch of snapd for UC16, then its good enough.

If the interface is missing from snapd (i don't see this likely, why would it be?), then we need to keep this test and fix that

Copy link
Member

@alfonsosanchezbeato alfonsosanchezbeato left a comment

Choose a reason for hiding this comment

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

+1 on @Meulengracht comments, otherwise it looks fine

Copy link
Member

@Meulengracht Meulengracht left a comment

Choose a reason for hiding this comment

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

reviewed and commented

Copy link
Member

@alfonsosanchezbeato alfonsosanchezbeato left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

This task removes tests executions for uc16 but the support remains
because we need to test core snap for security releases.

Other changes are included to move some tests from uc16 to uc18.
remodel-kernel to uc16-remodel-kernel
@sergiocazzolato sergiocazzolato merged commit e899805 into canonical:master Nov 15, 2024
52 of 56 checks passed
@sergiocazzolato sergiocazzolato deleted the tests-remove-uc16-support branch November 15, 2024 20:55
sespiros added a commit to sespiros/snapd that referenced this pull request Nov 19, 2024
* master: (44 commits)
  wrappers: do not reload activation units (canonical#14724)
  gadget/install: support for no{exec,dev,suid} mount flags
  interfaces/builtin/mount_control: add support for nfs mounts (canonical#14694)
  tests: use gojq - part 1 (canonical#14686)
  interfaces/desktop-legacy: allow DBus access to com.canonical.dbusmenu
  interfaces/builtin/fwupd.go: allow access to nvmem for thunderbolt plugin
  tests: removing uc16 executions (canonical#14575)
  tests: Added arm github runner to build snapd (canonical#14504)
  tests: no need to run spread when there are not tests matching the filter (canonical#14728)
  tests/lib/tools/store-state: exit on errors, update relevant tests (canonical#14725)
  tests: udpate the github workflow to run tests suggested by spread-filter tool (canonical#14519)
  testtime: add mockable timers for use in tests (canonical#14672)
  interface/screen_inhibit_control: Improve screen inhibit control for use on core (canonical#14134)
  tests: use images with 20G disk in openstack (canonical#14720)
  i/builtin: allow @ in custom-device filepaths (canonical#14651)
  tests: refactor test-snapd-desktop-layout-with-content
  tests: fix broken app definition
  tests: capitalize sentences in comments
  tests: wrap very long shell line
  tests: fix raciness in async startup and sync install
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants