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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

testing: Init plugin config when for tests #41533

Merged
merged 1 commit into from
Oct 21, 2020

Conversation

cpuguy83
Copy link
Member

@cpuguy83 cpuguy83 commented Oct 7, 2020

fixes #40932

This fixes a panic when running this test for me locally.


This is not panicing on CI because CI has TEST_SKIP_INTEGRATION_CLI=1 馃憥

This fixes a panic when running this test for me locally.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@thaJeztah
Copy link
Member

This is not panicing on CI because CI has TEST_SKIP_INTEGRATION_CLI=1 馃憥

FWIW, I think these steps should run test-integration-cli on amd64, but split into two parallel steps;

moby/Jenkinsfile

Lines 333 to 337 in ef6416f

# integration-cli first set
TEST_INTEGRATION_DEST=2 CONTAINER_NAME=${CONTAINER_NAME}-2 TEST_SKIP_INTEGRATION=1 TESTFLAGS="-test.run Test(DockerSuite|DockerNetworkSuite|DockerHubPullSuite|DockerRegistrySuite|DockerSchema1RegistrySuite|DockerRegistryAuthTokenSuite|DockerRegistryAuthHtpasswdSuite)/" run_tests &
# integration-cli second set
TEST_INTEGRATION_DEST=3 CONTAINER_NAME=${CONTAINER_NAME}-3 TEST_SKIP_INTEGRATION=1 TESTFLAGS="-test.run Test(DockerSwarmSuite|DockerDaemonSuite|DockerExternalVolumeSuite)/" run_tests &

However, because they try to make the split, it looks like DockerPluginSuite is not in that list 馃槥

First one is;

TESTFLAGS="-test.run Test(DockerSuite|DockerNetworkSuite|DockerHubPullSuite|DockerRegistrySuite|DockerSchema1RegistrySuite|DockerRegistryAuthTokenSuite|DockerRegistryAuthHtpasswdSuite)/"

Second one is

TESTFLAGS="-test.run Test(DockerSwarmSuite|DockerDaemonSuite|DockerExternalVolumeSuite)/" 

Checking what suites there are;

grep -rnw ./integration-cli/ -e 'type .*Suite'

./integration-cli//check_test.go:146:type DockerSuite struct {
./integration-cli//check_test.go:174:type DockerRegistrySuite struct {
./integration-cli//check_test.go:201:type DockerSchema1RegistrySuite struct {
./integration-cli//check_test.go:228:type DockerRegistryAuthHtpasswdSuite struct {
./integration-cli//check_test.go:257:type DockerRegistryAuthTokenSuite struct {
./integration-cli//check_test.go:292:type DockerDaemonSuite struct {
./integration-cli//check_test.go:331:type DockerSwarmSuite struct {
./integration-cli//check_test.go:390:type DockerPluginSuite struct {
./integration-cli//docker_hub_pull_suite_test.go:20:type DockerHubPullSuite struct {
./integration-cli//docker_cli_network_test.go:9:type DockerNetworkSuite struct {
./integration-cli//docker_cli_external_volume_driver_test.go:39:type DockerExternalVolumeSuite struct {

@thaJeztah
Copy link
Member

Looks like DockerPluginSuite is the only one missing; let me open a PR to add it, and that one should reproduce the problem

@thaJeztah
Copy link
Member

Opened #41559

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

ping @tiborvass @tianon ptal

@thaJeztah
Copy link
Member

ping @AkihiroSuda @tianon ptal

Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

simple enough; LGTM 馃槃

@thaJeztah
Copy link
Member

green enough as well; let's merge: thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TestDockerPluginSuite/TestPluginInstallArgs nil panic on s390x and ppc64le
3 participants