Skip to content
This repository was archived by the owner on Apr 3, 2018. It is now read-only.

Comments

test: move required test support files locally#660

Merged
sboeuf merged 4 commits intocontainers:masterfrom
grahamwhaley:20180305_testfiles_local
Mar 6, 2018
Merged

test: move required test support files locally#660
sboeuf merged 4 commits intocontainers:masterfrom
grahamwhaley:20180305_testfiles_local

Conversation

@grahamwhaley
Copy link
Contributor

Rather than have some of our required test support files live in /tmp,
let's install them in a local dir instead.

Graham whaley added 3 commits March 5, 2018 15:35
Rather than installing some parts we need into /tmp,
install them into a tree-local directory.

This is cleaner, and when you reboot your machine your
install files will not get cleaned away.

As the same time, only invoke sudo for the parts of the
script we really need to, which keeps the file perms of
the local parts more correct, and accessible by the user.

Signed-off-by: Graham whaley <graham.whaley@intel.com>
Now we have the test support files being set up in a local
dir, point the test code at them.

Signed-off-by: Graham whaley <graham.whaley@intel.com>
Now we place the test support files in a local dir, update
the api_test reference to use that path.

Fixes: containers#645

Signed-off-by: Graham whaley <graham.whaley@intel.com>
@grahamwhaley
Copy link
Contributor Author

Marked as DNM so we can discuss (so really an RFC).

  • the make check works as before
  • the go test -bench=. gives the same results as before (see below)

I didn't realise we had some benchmarks ;-) I managed to run them with:

$ sudo -E env PATH="$PATH" go test -bench=.

It looks like we may have bitrotted though (as I suspect nobody runs these). The output I have from both the original and the modified-in-this-patch I get is:

Some output on stderrERRO[0004] hook error                                    arch=amd64 error="exit status 1: stdout: , stderr: " hook-type=pre-start source=virtcontainers subsystem=hooks
Some output on stderrBenchmarkCreateStartStopDeletePodQemuHypervisorHyperstartAgentNetworkCNI-4              2         699418793 ns/op
BenchmarkCreateStartStopDeletePodQemuHypervisorNoopAgentNetworkCNI-4                   2      552637127 ns/op
BenchmarkCreateStartStopDeletePodQemuHypervisorHyperstartAgentNetworkNoop-4            3      532006579 ns/op
BenchmarkCreateStartStopDeletePodQemuHypervisorNoopAgentNetworkNoop-4                 10      251059396 ns/op
BenchmarkCreateStartStopDeletePodMockHypervisorNoopAgentNetworkNoop-4                500        4059463 ns/op
BenchmarkStartStop1ContainerQemuHypervisorHyperstartAgentNetworkNoop-4                 2     1117480785 ns/op
BenchmarkStartStop10ContainerQemuHypervisorHyperstartAgentNetworkNoop-4                1     6304859774 ns/op
PASS
ok      github.com/containers/virtcontainers    27.195s

@sboeuf - something you think we should fix? Any idea how?

mkdir -p ${tmpdir}/${virtcontainers_build_dir}

TMPDIR="/tmp"
TMPDIR="${SCRIPT_PATH}/supportfiles"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add a commit adding this whole directory to .gitignore ? This way, we won't have git tracking those files that we could add to a patch by mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(oops, I meant to but forgot...) done and pushed

Add the test support files dir (utils/supportfiles) to
gitignore, as we will never add those to the repo.

Signed-off-by: Graham whaley <graham.whaley@intel.com>
@sboeuf
Copy link
Collaborator

sboeuf commented Mar 5, 2018

LGTM

@sboeuf sboeuf merged commit a927fef into containers:master Mar 6, 2018
@sboeuf sboeuf removed the review label Mar 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants