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: various fixes & improvements to support packaging #117

Merged
merged 4 commits into from
Nov 29, 2023

Conversation

GabrielNagy
Copy link
Contributor

These changes were prompted by failures during Launchpad tests (and running tests on my own machine). Details are in the individual commits.

This will make for a clearer message if the test fails.
Instead of checking for a hardcoded "Dev", opt for the actual constant
to make the test pass when we override it during Debian packaging.
@GabrielNagy GabrielNagy force-pushed the fix-various-tests branch 2 times, most recently from 0a31376 to d30cce1 Compare November 29, 2023 11:30
@codecov-commenter
Copy link

codecov-commenter commented Nov 29, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (4093f4c) 87.92% compared to head (4fb41d5) 87.70%.

Files Patch % Lines
internal/cache/serialization.go 57.14% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #117      +/-   ##
==========================================
- Coverage   87.92%   87.70%   -0.23%     
==========================================
  Files          29       29              
  Lines        2096     2106      +10     
==========================================
+ Hits         1843     1847       +4     
- Misses        195      199       +4     
- Partials       58       60       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

On my machine root isn't an active user so the active user test fails
for me. Use the current user instead which is certain to be active, but
fallback on root if the user doesn't have a set name.
We don't have a running system bus in chroot environments (e.g.
Launchpad builders) so we need to create one ourselves.
@GabrielNagy GabrielNagy marked this pull request as ready for review November 29, 2023 11:39
@GabrielNagy GabrielNagy requested a review from a team as a code owner November 29, 2023 11:39
@GabrielNagy GabrielNagy merged commit 570a972 into main Nov 29, 2023
5 checks passed
@GabrielNagy GabrielNagy deleted the fix-various-tests branch November 29, 2023 13:28
@@ -62,12 +63,18 @@ func (c *Cache) dumpToYaml() (string, error) {
c.mu.RLock()
defer c.mu.RUnlock()

username := "root"
currentUser, _ := user.Current()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's still good to catch and return the error here and let the test deal with it.

@@ -104,6 +111,12 @@ func dbfromYAML(r io.Reader, destDir string) error {
return err
}

username := "root"
currentUser, _ := user.Current()
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here, return the error if any

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