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

make: pick up all privileged tests in make tests-privileged #10734

Merged
merged 3 commits into from Mar 27, 2020

Conversation

tklauser
Copy link
Member

As seen in #10729 some privileged tests currently aren't picked up by
the tests-privileged rule because it expects the privileged_tests
build tag to directly follow +build. However, some *_test.go files
use additional build tags before that. Account for that when evaluating
the privileged tests to run. At current HEAD this will pick up 3 more
privileged test packages:

% grep --include='*.go' -ril '+build privileged_tests' . | wc -l
16
% grep --include='*.go' -ril '+build [^!]*privileged_tests' . | wc -l
19

To be merged after #10729

/cc @joestringer @christarazi

@tklauser tklauser added pending-review release-note/misc This PR makes changes that have no direct user impact. labels Mar 27, 2020
@tklauser tklauser requested a review from a team as a code owner March 27, 2020 10:10
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Mar 27, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.6.9 Mar 27, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.7.2 Mar 27, 2020
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Nice find!

@coveralls
Copy link

coveralls commented Mar 27, 2020

Coverage Status

Coverage decreased (-0.03%) to 45.51% when pulling ebd4b18 on pr/tklauser/run-all-privileged-tests into 0b5e263 on master.

@tklauser tklauser force-pushed the pr/tklauser/run-all-privileged-tests branch from a81cf0f to 7c866ae Compare March 27, 2020 10:50
@tklauser tklauser requested a review from a team March 27, 2020 10:50
@tklauser
Copy link
Member Author

test-me-please

@tklauser
Copy link
Member Author

Cherry-picked @christarazi's fix from #10729 to be able to run tests (commit fe4bbd5e3430f3768190c63870ae3e8e538c0d9f in this PR).

christarazi and others added 3 commits March 27, 2020 17:56
This seems like an innocent mistake during #8954.
#8954 (comment)

Signed-off-by: Chris Tarazi <chris@isovalent.com>
As seen in #10729 some privileged tests currently aren't picked up by
the `tests-privileged` rule because it expects the `privileged_tests`
build tag to directly follow `+build`. However, some `*_test.go` files
use additional build tags before that. Account for that when evaluating
the privileged tests to run. At current `HEAD` this will pick up 3 more
privileged test packages:

```
% grep --include='*.go' -ril '+build privileged_tests' . | wc -l
16
% grep --include='*.go' -ril '+build [^!]*privileged_tests' . | wc -l
19
```

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Fix the follow test build errors:

```
 # github.com/cilium/cilium/pkg/sysctl [github.com/cilium/cilium/pkg/sysctl.test]
 pkg/sysctl/sysctl_linux_privileged_test.go:43:4: cannot use "1" (type string) as type []byte in field value
 pkg/sysctl/sysctl_linux_privileged_test.go:48:4: cannot use "1" (type string) as type []byte in field value
 pkg/sysctl/sysctl_linux_privileged_test.go:53:4: cannot use "1" (type string) as type []byte in field value
 pkg/sysctl/sysctl_linux_privileged_test.go:58:4: cannot use "1" (type string) as type []byte in field value
 pkg/sysctl/sysctl_linux_privileged_test.go:64:33: cannot use tc.value (type []byte) as type string in argument to writeSysctl
```

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@tklauser tklauser force-pushed the pr/tklauser/run-all-privileged-tests branch from 7c866ae to ebd4b18 Compare March 27, 2020 16:59
@tklauser tklauser requested a review from a team March 27, 2020 16:59
@tklauser
Copy link
Member Author

test-me-please

@tklauser
Copy link
Member Author

Needed to add an additional fix for the tests in pkg/sysctl: ebd4b18

/cc @pchaigno

Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

Nice! The build tags haven't been very fun to deal with as a newcomer :/

@borkmann borkmann merged commit 3486b3f into master Mar 27, 2020
1.8.0 automation moved this from In progress to Merged Mar 27, 2020
@borkmann borkmann deleted the pr/tklauser/run-all-privileged-tests branch March 27, 2020 21:20
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.7 in 1.7.2 Mar 30, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.6 in 1.6.9 Mar 30, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.6 in 1.6.9 Mar 30, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.7 to Backport done to v1.7 in 1.7.2 Apr 1, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.6 to Backport done to v1.6 in 1.6.9 May 13, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.6 to Backport done to v1.6 in 1.6.9 May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.6.9
Backport done to v1.6
1.7.2
Backport done to v1.7
1.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

7 participants