snap-exec: update tests to follow main_test pattern #4006

Merged
merged 13 commits into from Oct 10, 2017

Conversation

Projects
None yet
5 participants
Collaborator

mvo5 commented Oct 5, 2017

The snap-exec tests were using the same package as the snap-exec
main application. Most other packages in snapd use a $foo, $foo_test
package name pattern. This patch updates the tests to do this now.

Based on #3995 where I noticed that we use the old pattern.

mvo5 added some commits Sep 30, 2017

snap-exec: update tests to follow main_test pattern
The snap-exec tests were using the same package as the snap-exec
main application. Most other packages in snapd use a $foo, $foo_test
package name pattern. This patch updates the tests to do this now.

zyga approved these changes Oct 5, 2017

Looks good, some ideas about the test helper functions but not a blocker.

+func SetOptsCommand(s string) {
+ opts.Command = s
+}
+func GetOptsCommand() string {
@zyga

zyga Oct 5, 2017

Contributor

Maybe just OptsCommand?

+func SetOptsHook(s string) {
+ opts.Hook = s
+}
+func GetOptsHook() string {
@zyga

zyga Oct 5, 2017

Contributor

Ditto, maybe just OptsHook?

cmd/snap-exec/main.go
+ // snap/validate.go:appContentWhitelist)
+ tmpArgv := strings.Split(cmdAndArgs, " ")
+ cmd := tmpArgv[0]
+ cmdArgs := expandEnvCmdArgs(tmpArgv[1:], osutil.EnvMap(env))
@zyga

zyga Oct 5, 2017

Contributor

Nice, thank you :)

@@ -45,20 +46,19 @@ var _ = Suite(&snapExecSuite{})
func (s *snapExecSuite) SetUpTest(c *C) {
// clean previous parse runs
- opts.Command = ""
- opts.Hook = ""
+ snapExec.SetOptsCommand("")
@zyga

zyga Oct 5, 2017

Contributor

Do you need to undo that anywhere? I don't suppose so (just testing) but perhaps a MockCommandAndHook thing would suffice?

Contributor

zyga commented Oct 5, 2017

The test failure looks real @mvo5

+ echo 'Test that snap run use environments'
Test that snap run use environments
+ MATCH '^/var/snap'
+ basic-run.echo-data
cannot snap-exec: cannot exec "/snap/basic-run/x1/bin/echo": permission denied

mvo5 and others added some commits Oct 5, 2017

tests: don't remove built snap
Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
tests: make basic-run.echo executable
Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>

codecov-io commented Oct 9, 2017

Codecov Report

Merging #4006 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4006      +/-   ##
==========================================
+ Coverage   75.71%   75.71%   +<.01%     
==========================================
  Files         429      429              
  Lines       36709    36709              
==========================================
+ Hits        27793    27795       +2     
+ Misses       6964     6962       -2     
  Partials     1952     1952
Impacted Files Coverage Δ
cmd/snap-exec/main.go 69.4% <100%> (ø) ⬆️
overlord/ifacestate/helpers.go 63% <0%> (+0.66%) ⬆️

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 61ee255...c722a51. Read the comment docs.

Contributor

zyga commented Oct 9, 2017

@mvo5 I fixed the tests failures, I'll let you decide if anything else needs adjustments.

change lgtm

chipaca added some commits Oct 9, 2017

@chipaca chipaca merged commit d023f13 into snapcore:master Oct 10, 2017

4 of 7 checks passed

artful-i386 autopkgtest finished (failure)
Details
xenial-amd64 autopkgtest finished (failure)
Details
xenial-ppc64el autopkgtest finished (failure)
Details
artful-amd64 autopkgtest finished (success)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-i386 autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment