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/nested/manual/refresh-revert-fundamentals: fix variable use #10722

Merged

Conversation

bboozzoo
Copy link
Collaborator

@bboozzoo bboozzoo commented Sep 2, 2021

Commit a7657b5 seems to have broken the test,
which went unnoticed until there happened to be leaking a set -u from one of the
sourced files which triggered this error:

+ echo 'Check the new version of the snaps is correct after the system reboot'
Check the new version of the snaps is correct after the system reboot
+ tests.nested exec 'snap list core20'
+ MATCH '^core20.*1113.*latest/edge.*'
Warning: Permanently added '[localhost]:8022' (ECDSA) to the list of known hosts.
+ echo 'Check the change is completed'
Check the change is completed
+ case "$SNAP" in
/bin/bash: line 121: REVERT_ID: unbound variable
-----

Signed-off-by: Maciej Borzecki maciej.zenon.borzecki@canonical.com

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

Commit a7657b5 seems to have broken the test,
which went unnoticed until there happened to be leaking a set -u from one of the
sourced files which triggered this error:

```
+ echo 'Check the new version of the snaps is correct after the system reboot'
Check the new version of the snaps is correct after the system reboot
+ tests.nested exec 'snap list core20'
+ MATCH '^core20.*1113.*latest/edge.*'
Warning: Permanently added '[localhost]:8022' (ECDSA) to the list of known hosts.
+ echo 'Check the change is completed'
Check the change is completed
+ case "$SNAP" in
/bin/bash: line 121: REVERT_ID: unbound variable
-----
```

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
@bboozzoo bboozzoo added Simple 😃 A small PR which can be reviewed quickly Run nested The PR also runs tests inluded in nested suite labels Sep 2, 2021
@codecov-commenter
Copy link

codecov-commenter commented Sep 2, 2021

Codecov Report

Merging #10722 (a16424c) into master (df2a582) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10722      +/-   ##
==========================================
- Coverage   78.36%   78.35%   -0.01%     
==========================================
  Files         885      885              
  Lines       99526    99526              
==========================================
- Hits        77991    77987       -4     
- Misses      16636    16639       +3     
- Partials     4899     4900       +1     
Flag Coverage Δ
unittests 78.35% <ø> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
src/github.com/snapcore/snapd/osutil/synctree.go 76.41% <0.00%> (-2.84%) ⬇️
...hub.com/snapcore/snapd/cmd/snap/cmd_debug_state.go 70.18% <0.00%> (-0.46%) ⬇️
...apcore/snapd/interfaces/builtin/network_control.go 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df2a582...a16424c. Read the comment docs.

Copy link
Contributor

@mardy mardy left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@stolowski stolowski left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Trying to pass the right arguments to the retry tool on the nested host the
quoting gets really complicated. Revert to invoking retry from the test host.

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Copy link
Collaborator

@sergiocazzolato sergiocazzolato left a comment

Choose a reason for hiding this comment

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

Nice, thanks

@bboozzoo
Copy link
Collaborator Author

bboozzoo commented Sep 3, 2021

Failures are unrelated, @mvo5 can you land this PR?

@mvo5 mvo5 merged commit f50b1dc into snapcore:master Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Run nested The PR also runs tests inluded in nested suite Simple 😃 A small PR which can be reviewed quickly
Projects
None yet
6 participants