-
Notifications
You must be signed in to change notification settings - Fork 574
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: replace nfs support test with generalized cifs test #14562
tests: replace nfs support test with generalized cifs test #14562
Conversation
Signed-off-by: Zygmunt Krynicki <[email protected]>
This test is much more modern and easier to reason about. It is also somewhat different in that it doesn't really exericse all the various configurations where nfs/cifs is detected, that is handled with unit tests (and we know the detection part is relatively weak). Instead the test focuses that a given user may be "remote home" and that the special permissions work as expected. Signed-off-by: Zygmunt Krynicki <[email protected]>
This test is now replaced by generalized cifs-support test. Not as many systems are covered but the test is much more robust and easier to work with. Signed-off-by: Zygmunt Krynicki <[email protected]>
Signed-off-by: Zygmunt Krynicki <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #14562 +/- ##
==========================================
+ Coverage 78.85% 78.87% +0.01%
==========================================
Files 1079 1083 +4
Lines 145615 146105 +490
==========================================
+ Hits 114828 115239 +411
- Misses 23601 23671 +70
- Partials 7186 7195 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
LGTM
tests.cleanup defer tests.backup restore /etc/samba | ||
# NOTE: due to <<- and the fact that this is shell-in-yaml, the spaces | ||
# are followed by tabs. | ||
cat <<-__SMB__ >>'/etc/samba/smb.conf' |
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.
is it removed with samba pkg right?
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 think it is only removed on purge.
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, just minor comments inline
Packages are automatically removed by a higher-level restore logic. Signed-off-by: Zygmunt Krynicki <[email protected]>
The cifs test was created relatively recently to test how a single home directory is mounted using CIFS and how a user using that home directory can use snaps. We had an earlier test for NFS with some more fragile and complex setup.
Given that NFS test fails periodically and it was broken for a while on some systems, replace the old NFS test with a variant of the CIFS test. Most of the existing test is kept intact. The portion that manages the user account is moved to the front to be easily shared with the conditional bits below.
The patch series contains a new toy executable called
okfail
which indicates that a command is allowed to fail without consequences. It should be moved to the test helpers project but it can be done on the next sync.Jira: https://warthogs.atlassian.net/browse/SNAPDENG-31425