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: adding extra scenarios for fault injection #14489

Conversation

sergiocazzolato
Copy link
Collaborator

This change includes some scenarios that are missing to check fault injection.

In this case are included fault/reboot for:

  • install snapd --dangerous
  • remodel
  • install kernel component
  • update boot config

@sergiocazzolato sergiocazzolato added the Run nested The PR also runs tests inluded in nested suite label Sep 11, 2024
@github-actions github-actions bot added the Run Nested -auto- Label automatically added in case nested tests need to be executed label Sep 11, 2024
This change includes some scenarios that are missing to check fault
injection.

In this case are included fault/reboot for:
- install snapd --dangerous
- remodel
- install kernel component
- update boot config
Copy link
Contributor

@bboozzoo bboozzoo left a comment

Choose a reason for hiding this comment

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

LGTM

One more idea about possible scenarios is a transaction updating both the base snap and snapd. Depending on how far has the installation process advanced a failure should either roll back the complete thing, or finish updating both snaps.

@sergiocazzolato sergiocazzolato changed the title tests: adding missing scenarios for fault injection tests: adding extra scenarios for fault injection Sep 19, 2024
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.

I have a comment on what is the expected outcome of the tests

Copy link

codecov bot commented Sep 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.87%. Comparing base (04d0ab0) to head (f179add).
Report is 59 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #14489      +/-   ##
==========================================
+ Coverage   78.83%   78.87%   +0.03%     
==========================================
  Files        1078     1080       +2     
  Lines      145096   145826     +730     
==========================================
+ Hits       114389   115017     +628     
- Misses      23546    23617      +71     
- Partials     7161     7192      +31     
Flag Coverage Δ
unittests 78.87% <ø> (+0.03%) ⬆️

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.

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, although I have a question

Comment on lines +12 to +13
#TAG/gadget_panic_config_bootloader: update-config-bootloader
#FAULT/gadget_panic_config_bootloader: panic

Choose a reason for hiding this comment

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

Is this the one that finishes with error? Maybe worth adding a comment on why it is commented out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, this is an scenario that cannot be reproduced, I'll work on that one in a following pr, I need @valentindavid for that

@sergiocazzolato sergiocazzolato merged commit d7f8099 into canonical:master Sep 27, 2024
53 of 56 checks passed
@sergiocazzolato sergiocazzolato deleted the tests-add-missing-fault-injection branch September 27, 2024 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Run Nested -auto- Label automatically added in case nested tests need to be executed Run nested The PR also runs tests inluded in nested suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants