-
Notifications
You must be signed in to change notification settings - Fork 49
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
remove deb package blob used for testing #113
Conversation
To make the content transparent and facilitate changing the package, this commit creates the deb used for testing programmatically instead of embedding it as a base64-encoded blob directly.
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.
The passing tests are assuring me that the necessary files are intact since we check modes and hashes in the tests.
Left a nit comment about the name below. Otherwise, looks overall good to me.
internal/testutil/pkgdata.go
Outdated
abj2Z7KoYMBXz9dwNNP2Aw13FguKkogezW5cqy4lCg== | ||
` | ||
func init() { | ||
PackageData["base-files"] = MustMakeDeb(baseFilesPackageEntries) |
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 wonder if we should keep calling it base-files
since it contains file not found in traditional base-files
packages. For example, the /usr/bin/hello
script. And also lacks a few files (and dirs) from the usual base-files packages.
Since we are refactoring, how about renaming the entry to something like test-package
?
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.
There has been a change of plan, we are no longer going to change base-files but the tests instead. Gustavo had a good point about the previous version of the PR not being the best solution, either we use a real package or we create tests data specifically.
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.
Looks good to me, thanks! I just left a nitpick comment.
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, that's looking nice. Just superficial comments.
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!
To make the content transparent and facilitate testing by creating the minimal working package, this PR changes the tests to avoid relying on base files, instead creating the package for the tests explicitly. The new package contains only the relevant testing data.