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

Doc: Update the unit test section for privileged tests #11433

Conversation

soumynathan
Copy link
Contributor

This PR adds instructions to the cilium documentation on how to
run the 'privileged' unit tests.

Fixes: #11431
Signed-off-by: Swaminathan Vasudevan svasudevan@suse.com

@soumynathan soumynathan requested a review from a team as a code owner May 8, 2020 19:37
@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 May 8, 2020
@soumynathan soumynathan force-pushed the update-doc-to-add-instructions-to-run-privileged-unit-tests branch from cf9adcb to bb155ae Compare May 8, 2020 19:41
@coveralls
Copy link

coveralls commented May 8, 2020

Coverage Status

Coverage decreased (-0.02%) to 36.931% when pulling 39d3abc on soumynathan:update-doc-to-add-instructions-to-run-privileged-unit-tests into 9168268 on cilium:master.

Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Thanks for the improvement to the docs!

I mentioned an easier way to test just one package below, at least for me it's easier to remember and the variable is consistent for both the unprivileged and the privileged tests.

Documentation/contributing/testing/unit.rst Outdated Show resolved Hide resolved
@joestringer joestringer added the release-note/misc This PR makes changes that have no direct user impact. label May 8, 2020
@soumynathan soumynathan force-pushed the update-doc-to-add-instructions-to-run-privileged-unit-tests branch from bb155ae to 14a4fea Compare May 8, 2020 20:54
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.

Thanks for the PR! Minor nits

Documentation/contributing/testing/unit.rst Outdated Show resolved Hide resolved
Documentation/contributing/testing/unit.rst Outdated Show resolved Hide resolved
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.

Thanks for the improvement @soumynathan! A couple editing nits below.

Documentation/contributing/testing/unit.rst Outdated Show resolved Hide resolved
Documentation/contributing/testing/unit.rst Outdated Show resolved Hide resolved
Documentation/contributing/testing/unit.rst Outdated Show resolved Hide resolved
@soumynathan soumynathan force-pushed the update-doc-to-add-instructions-to-run-privileged-unit-tests branch 2 times, most recently from a2201d6 to f7fa64c Compare May 8, 2020 21:57
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.

Minor nit and should be good to go!

Documentation/contributing/testing/unit.rst Outdated Show resolved Hide resolved
This PR adds instructions to the cilium documentation on how to
run the 'privileged' unit tests.

Fixes: cilium#11431
Signed-off-by: Swaminathan Vasudevan <svasudevan@suse.com>
@soumynathan soumynathan force-pushed the update-doc-to-add-instructions-to-run-privileged-unit-tests branch from f7fa64c to 39d3abc Compare May 27, 2020 16:19
@maintainer-s-little-helper
Copy link

Commit dfa206b18f8204fd4a7300a816a2c07fe00282b8 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 27, 2020
@christarazi
Copy link
Member

christarazi commented May 27, 2020

I'm not sure what just happened right there. I clicked "Rebase and merge" and GH's UI just bugged out and made a strange merge commit. @soumynathan Do you mind removing that commit from the PR? I will merge after.

Edit: Looks like @pchaigno was able to handle it. Merging

@pchaigno pchaigno force-pushed the update-doc-to-add-instructions-to-run-privileged-unit-tests branch from dfa206b to 39d3abc Compare May 27, 2020 17:31
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 27, 2020
@christarazi christarazi merged commit e868f36 into cilium:master May 27, 2020
1.8.0 automation moved this from In progress to Merged May 27, 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.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

doc: Update the unit test section for privileged tests
5 participants