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

validation/linux_cgroups_*: Generate TAP output (and outside-validation cleanup) #542

Merged
merged 3 commits into from
Dec 29, 2017

Conversation

wking
Copy link
Contributor

@wking wking commented Dec 13, 2017

The test harness requires TAP output. Before this pull request:

# RUNTIME=runc tap ./validation/linux_cgroups_pids.t
./validation/linux_cgroups_pids.t ..................... 0/1
  Skipped: 1
    ./validation/linux_cgroups_pids.t no tests found

total ................................................. 0/1

  0 passing (191.66ms)
  1 pending

After this pull request:

# RUNTIME=runc tap ./validation/linux_cgroups_pids.t
./validation/linux_cgroups_pids.t ..................... 3/3
total ................................................. 3/3

  3 passing (185.118ms)

  ok

This pull request also:

  • DRYs up some relative/absolute tests by factoring out the shared code into util.ValidateLinuxResources* helpers.
  • Adds a missing AddLinuxResourcesNetworkPriorities call; previously we were testing for a priority that we hadn't actually configured.
  • Fixes a startcreate typo in an RuntimeOutsideValidate error message.
  • Adjusts the RuntimeOutsideValidate to be generic; passing:
    • the full state object instead of just the PID, and
    • the full config object instead of just the cgroups path.

All of these are cleanups to the implementation which just landed via #93.

Pass:

* The full state object instead of just the PID, and
* The full config object instead of just the cgroups path

in case other properties are useful for testing (which I expect will
happen once RuntimeOutsideValidate is used for more than cgroups).

Signed-off-by: W. Trevor King <wking@tremily.us>
This is right after the r.Create() call, so match the error message
used after r.Create in RuntimeInsideValidate.

Signed-off-by: W. Trevor King <wking@tremily.us>
@Mashimiao
Copy link

@wking please fix your patch refer to CI logs

@wking wking force-pushed the outside-validation-cleanup branch 2 times, most recently from 6a23d21 to 3bd8d43 Compare December 14, 2017 15:35
The test harness requires TAP output.  Before this commit:

  # RUNTIME=runc tap ./validation/linux_cgroups_pids.t
  ./validation/linux_cgroups_pids.t ..................... 0/1
    Skipped: 1
      ./validation/linux_cgroups_pids.t no tests found

  total ................................................. 0/1

    0 passing (191.66ms)
    1 pending

After this commit:

  # RUNTIME=runc tap ./validation/linux_cgroups_pids.t
  ./validation/linux_cgroups_pids.t ..................... 3/3
  total ................................................. 3/3

    3 passing (185.118ms)

    ok

This commit also DRYs up some relative/absolute tests by factoring out
the shared code into util.ValidateLinuxResources* helpers.

And it adds a missing AddLinuxResourcesNetworkPriorities call;
previously we were testing for a priority that we hadn't actually
configured.

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking
Copy link
Contributor Author

wking commented Dec 14, 2017

please fix your patch refer to CI logs

Done. Thanks for the ping.

@zhouhao3
Copy link

zhouhao3 commented Dec 25, 2017

LGTM

Approved with PullApprove

1 similar comment
@liangchenye
Copy link
Member

liangchenye commented Dec 29, 2017

LGTM

Approved with PullApprove

@liangchenye liangchenye merged commit 56c456d into opencontainers:master Dec 29, 2017
@wking wking deleted the outside-validation-cleanup branch January 11, 2018 05:21
wking added a commit to wking/ocitools-v2 that referenced this pull request Apr 5, 2018
I'd accidentally committed it in 3bd8d43 (validation/linux_cgroups_*:
Generate TAP output, 2018-12-13, opencontainers#542).

Signed-off-by: W. Trevor King <wking@tremily.us>
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.

None yet

4 participants