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

Jump to bottom

testing: API for deferring a cleanup step when subtests use Parallel #31651

Closed
RaduBerinde opened this issue Apr 24, 2019 · 3 comments
Closed

testing: API for deferring a cleanup step when subtests use Parallel #31651

RaduBerinde opened this issue Apr 24, 2019 · 3 comments
Labels
FrozenDueToAge Proposal

Comments

@RaduBerinde
Copy link
Contributor

A relatively common pattern is to run a test that has multiple subtests that can be run in parallel:

func TestSomething(t *testing.T) {
  t.Run("case1", func (t *testing.T) { t.Parallel(); ... })
  t.Run("case2", func (t *testing.T) { t.Parallel(); ... })
}

But what if the parent test needs to set something up and clean it up at the end? You'd be tempted to do:

func TestSomething(t *testing.T) {
  setup()
  defer cleanup()

  t.Run("case1", func (t *testing.T) { t.Parallel(); ... })
  t.Run("case2", func (t *testing.T) { t.Parallel(); ... })
}

Unfortunately this is not correct - cleanup will run too early, before the subtests finish (in fact, before they even get passed the Parallel() call).

Most CockroachDB tests use a defer leaktest.AfterTest(t)() call in the beginning that checks for stray goroutines; using Parallel() in subtests breaks that pattern.

My proposal is to add an API to T that runs a cleanup step after all subtests finish; defer cleanup() would become t.Defer(cleanup()) or similar.

Also, this gotcha should be called out in the documentation (#23368 is related).
@RaduBerinde RaduBerinde mentioned this issue Apr 24, 2019
Merged
@RaduBerinde
Copy link
Contributor Author

Ah.. I found this in the higher-level documentation:

func TestTeardownParallel(t *testing.T) {
    // This Run will not return until the parallel tests finish.
    t.Run("group", func(t *testing.T) {
        t.Run("Test1", parallelTest1)
        t.Run("Test2", parallelTest2)
        t.Run("Test3", parallelTest3)
    })
    // <tear-down code>
}

This is a good workaround for this problem (though it would still be better if there was an explicit API). Either way, I think there should be a pointer to this information in the documentation of Parallel.

@katiehockman
Copy link
Contributor

/cc @mpvl

timflannagan added a commit to timflannagan/operator-metering that referenced this issue Oct 16, 2019
@timflannagan
test/e2e: Refactor the e2e package to use the deploy metering package.
0fc7833 
With new support for deploying metering using a Golang package (and CLI tool), we can transistion towards removing the hack scripts that deploy metering, and reduce the amount of wrapper scripts used to provision the testing environment. This commit uses the deployframework package to deploy metering, then run the report tests.

Note: we needed to encapsulate the testReportingProducesData test helper function with t.Run in order to respect the defer closure call, and avoid removing the namespace while parallel tests paused/ran/continued.

For more information on the behavior described above, see:
[1] golang/go#22993
[2] golang/go#31651
chancez pushed a commit to chancez/operator-metering that referenced this issue Nov 8, 2019
@timflannagan
Squashed commit of the following:
d60c6fe 
commit ceeb577
Merge: 71604bf 5991846
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Thu Nov 7 19:30:36 2019 +0100

    Merge pull request kube-reporting#1020 from timflannagan1/update-local-log-names

    test/deployframework: Create the local operator logs without a trailing suffix.

commit 5991846
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Tue Nov 5 16:09:24 2019 -0500

    test/deployframework: Create the local operator logs without a trailing suffix.

commit 71604bf
Merge: eecd156 8983e68
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Wed Nov 6 21:14:43 2019 +0100

    Merge pull request kube-reporting#1011 from timflannagan1/add-invalid-metering-config-test

    test: Add support for testing an invalid MeteringConfig.

commit 8983e68
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Wed Nov 6 13:42:18 2019 -0500

    test: Validate the expected error message regex matches the actual error regex.

    Due to potential differing kube versions, the error message returned from attempting to install an invalid MeteringConfig object can state different 'required value' messages.

commit 7f442e3
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Fri Nov 1 12:53:01 2019 -0400

    test: Add support for testing an invalid MeteringConfig.

commit 26ce295
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Fri Oct 18 12:46:31 2019 -0400

    cmd,pkg/deploy: Remove the nested logging and fix linting errors.

    With a recent PR failing e2e [1], the following error message was returned: 'Failed to deploy metering: Failed to uninstall metering: Failed to delete the MeteringConfig resource: Failed to delete the MeteringConfig resource:' which is difficult to read/follow the stack/command trace.

    [1] https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/operator-framework_operator-metering/997/pull-ci-operator-framework-operator-metering-feature-metering-e2e-aws/62

commit eecd156
Merge: 06334d0 b010a15
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Wed Nov 6 17:31:45 2019 +0100

    Merge pull request kube-reporting#1018 from timflannagan1/update-test-names

    test/e2e: Rename the reporting test files to reflect their names in the test table.

commit b010a15
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Tue Nov 5 14:51:18 2019 -0500

    test/e2e: Rename the reporting test file names to reflect their naming in the test table.

commit 06334d0
Merge: 3c7f96d 3c0d97e
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Tue Nov 5 22:08:29 2019 +0100

    Merge pull request kube-reporting#1005 from timflannagan1/support-e2e-local

    Support running the e2e suite locally.

commit 3c0d97e
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Mon Nov 4 14:02:22 2019 -0500

    test: Update the e2e and deployframework packages to support running the e2e suite locally.

    This commit addresses the need to run the e2e suite locally by deploying both the metering and reporting operator locally. In order to this, the deployframework package was extended to support another type that handles the current local context and performs cleanup on any resources provisioned (process IDs, tmp directories, etc.) while running the tests locally.

    The test/e2e/main_test.go was also updated to pass a set of extra environment variables that are used when running the reporting-operator locally (but can be lightly refactored to pass to the metering-operator as well). This was needed as the reporting tests have differing behavior in how metrics are imported/managed.

commit 1d6f3c3
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Mon Nov 4 14:00:52 2019 -0500

    hack: Update the various go test flags and unexport variables in e2e.sh.

commit 4ccb254
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Mon Nov 4 13:58:18 2019 -0500

    hack: Avoid using the shorthand symbol for the redirection of stderr to stdout.

    This was problematic when the e2e suite was run locally on older bash versions.

commit 87be25a
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Mon Nov 4 13:52:14 2019 -0500

    hack: Update the trap condition in e2e.sh to only handle the exit pseudo-signal.

commit 3bd02b9
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Mon Nov 4 13:46:39 2019 -0500

    cmd,pkg/deploy: Add support for skipping the installation of the metering deployment.

commit 32c209d
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Mon Nov 4 13:44:39 2019 -0500

    Makefile: Consolidate the deploy operator local variables in the e2e-local target.

commit 3c7f96d
Merge: 44719f4 c44e127
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Mon Oct 28 22:41:12 2019 +0100

    Merge pull request kube-reporting#992 from timflannagan1/integration-rewrite

    hack,test,Makefile: Consolidate the e2e and integration testing suites.

commit c44e127
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Thu Oct 17 15:08:01 2019 -0400

    test: Consolidate the integration package into the e2e package.

    This would remove the test/integration directory, and move the tests and testdata from the integration package, into the e2e package. Before, we had a lot of redundancy between the two packages, and we can now consolidate these two packages now that the deployframework package has been added.

    This would also update the e2e/*_tests.go files in order to accommodate the integration sub-tests and update the main_test.go to run these new sub-tests.

commit 2dae81f
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Thu Oct 17 17:40:02 2019 -0400

    test: Stop creating the cliensets in the reportingframework constructor.

    The deployframework constructor creates these clientsets already, so we can update the reportingframework constructor parameters to pass these clientsets directly.

commit a339799
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Thu Oct 17 17:09:32 2019 -0400

    Makefile: Update the integration target to do nothing.

    This is necessary as the metering configuration files stored in openshift/release still run the `make integration` command, so this is a temporary solution until the release repo has been updated.

commit 8e43643
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Wed Oct 23 13:52:41 2019 -0400

    hack: Update the timeout period in e2e.sh.

commit c0bfeb8
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Thu Oct 17 11:27:24 2019 -0400

    hack: Remove the deploy and test wrapper scripts.

    As we move the integration package into the e2e package, we no longer have any need for the deploy scripts (due to the addition of the deployframework package) and we can eliminate all the test wrapper scripts, and instead use hack/e2e.sh to do any setup necessary before running e2e tests.

commit 1a5ef88
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Thu Oct 17 16:28:02 2019 -0400

    hack: Remove the integration package from unit.sh.

commit 851fb9a
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Sat Oct 19 14:44:10 2019 -0400

    hack: Update e2e.sh to capture the exit code returned from the go test command.

commit 44719f4
Merge: 4372253 97568ae
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Tue Oct 22 00:33:17 2019 +0200

    Merge pull request kube-reporting#1001 from chancez/deployer_framework_updates

    Add unit test ensuring metering-operator and reporting-operator image repo/tags are set correctly

commit 97568ae
Author: Chance Zibolski <czibolsk@redhat.com>
Date:   Mon Oct 21 14:13:40 2019 -0700

    test/deployframework: Add unit test for NewDeployerConfig

commit 7511793
Author: Chance Zibolski <czibolsk@redhat.com>
Date:   Mon Oct 21 14:12:59 2019 -0700

    test/deployframework,test/e2e: Move creation of deploy.Config into separate method on DeployFramework

    For easier unit testing

commit 3e53de8
Author: Chance Zibolski <czibolsk@redhat.com>
Date:   Mon Oct 21 14:10:04 2019 -0700

    test: Extract deployframework methods into helper functions

commit 9396901
Author: Chance Zibolski <czibolsk@redhat.com>
Date:   Mon Oct 21 13:58:04 2019 -0700

    pkg,test: Update import aliases to be clearer between client and types packages

commit 5edb9b3
Author: Chance Zibolski <czibolsk@redhat.com>
Date:   Mon Oct 21 11:51:08 2019 -0700

    test: Remove fields on Deployframework that aren't used in methods

    Instead pass them as arguments. Also, make all configurable values into
    flags as well as allow setting them via environment variables.

commit 4372253
Merge: ad8489f 696f5c7
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Fri Oct 18 20:31:24 2019 +0200

    Merge pull request kube-reporting#994 from timflannagan1/ensure-reporting-operator-image

    test/deployframework: Ensure the overrided reporting-operator image isn't empty.

commit 696f5c7
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Thu Oct 17 17:55:13 2019 -0400

    test/deployframework: Ensure the metering and reporting operator images are properly set.

    As we push towards refactoring the e2e/integration suites, we need to ensure that overriding the reporting-operator images works as intended.

    This commit adds a testhelper function that validates if the image repository or tag are empty, when that image is overrided.

commit ad8489f
Merge: 2c8a5bb 0fc7833
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Thu Oct 17 01:12:43 2019 +0200

    Merge pull request kube-reporting#979 from timflannagan1/refactor-e2e-v2

    test,hack: Refactor the e2e package to use the deploy metering package.

commit 0fc7833
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Sat Oct 12 14:25:17 2019 -0400

    test/e2e: Refactor the e2e package to use the deploy metering package.

    With new support for deploying metering using a Golang package (and CLI tool), we can transistion towards removing the hack scripts that deploy metering, and reduce the amount of wrapper scripts used to provision the testing environment. This commit uses the deployframework package to deploy metering, then run the report tests.

    Note: we needed to encapsulate the testReportingProducesData test helper function with t.Run in order to respect the defer closure call, and avoid removing the namespace while parallel tests paused/ran/continued.

    For more information on the behavior described above, see:
    [1] golang/go#22993
    [2] golang/go#31651

commit a0f89f0
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Sat Oct 12 14:24:19 2019 -0400

    test/deployframework: Add the deployframework package.

commit 76961b1
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Wed Oct 16 17:45:57 2019 -0400

    hack: Add script responsible for logging container and resouce output to specified files.

commit 0e2eafd
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Wed Oct 16 17:45:36 2019 -0400

    hack: Update e2e.sh to avoid calling the test wrapper scripts.

commit 2c8a5bb
Merge: f318c5c 1e34341
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Wed Oct 16 21:23:31 2019 +0200

    Merge pull request kube-reporting#984 from timflannagan1/deploy-extra-labels

    pkg/deploy: Support passing extra namespace labels for use in testing.

commit 1e34341
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Tue Oct 15 18:16:12 2019 -0400

    pkg/deploy: Support passing extra namespace labels for use in testing.

commit f318c5c
Merge: 7d3402a f396d88
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Mon Oct 14 22:36:00 2019 +0200

    Merge pull request kube-reporting#981 from timflannagan1/stat-report-output-dir

    test/reportingframework: Stat the report output directory.

commit 7d3402a
Merge: 506cf02 82f7ec2
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Mon Oct 14 20:57:11 2019 +0200

    Merge pull request kube-reporting#980 from timflannagan1/hive-spec-update

    pkg/apis: Update various fields in the MeteringConfig API.

commit f396d88
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Sat Oct 12 14:26:42 2019 -0400

    test/reportingframework: Stat the report output directory.

    As we move the process of deploying metering away from the bash hack scripts, and into the testing code, we need to ensure that this directory exists before running any reporting tests. Before, we created the report output directory in the code via `os.MkdirAll(reportOutputDir, 0777)`, getting the value of `reportOutputDir` from an environment variable, and validating that environment variable was set/non-empty.

commit 82f7ec2
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Fri Oct 11 11:23:50 2019 -0400

    pkg/apis: Update various fields in the MeteringConfig API.
chancez pushed a commit to chancez/operator-metering that referenced this issue Nov 8, 2019
@timflannagan
Squashed commit of the following:
60aaae4 
commit 2c796ae3b65b3a067d99797738bc6ab3f9820236
Merge: ceeb577 8ce9bbe
Author: Chance Zibolski <czibolsk@redhat.com>
Date:   Fri Nov 8 12:59:38 2019 -0800

    Merge branch 'master' into feature-e2e-rewrite

    * master: (22 commits)
      images/metering-ansible-operator: Verify that the s3 credentials secret exists.
      Dockerfile.metering-ansible-operator*: set -x before yum commands
      Documentation/dev: Clarify differences in app registry namespaces better
      pkg/operator: Obey FileFormat in HiveTable resources
      pkg/operator: Fix managing partitions of HiveTables
      charts/openshift-metering: Add aws-billing datasource as input to aws-ec2-billing-raw query
      pkg/operator: Fix checking storageLocation database name for AWS billing datasources
      charts/openshift-metering: Update aws-ec2-billing datasource name and queryname
      pkg/operator/reporting: Fix quoting of partition values and partition location
      pkg/operator: Fix AWS partition values column names
      charts,manifests: Fix HiveTable partitions partitionSpec validation
      Update aws-sdk-go
      cmd,pkg/deploy: Remove the nested logging and fix linting errors.
      charts,manifests: Support configuring Presto hive.s3.use-instance-credentials Hive catalog config property
      charts,manifests: Support configuring reporting-operator securityContext
      charts/openshift-metering: Support annotations on service accounts
      charts/openshift-metering: Fix setting default fileformat for hive tables
      charts,manifests: Remove defaultCompression it's unused
      hack: Change junit report file name for e2e/integration tests
      hack: Fix mirroring to work for digests
      ...

commit ceeb577
Merge: 71604bf 5991846
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Thu Nov 7 19:30:36 2019 +0100

    Merge pull request kube-reporting#1020 from timflannagan1/update-local-log-names

    test/deployframework: Create the local operator logs without a trailing suffix.

commit 5991846
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Tue Nov 5 16:09:24 2019 -0500

    test/deployframework: Create the local operator logs without a trailing suffix.

commit 71604bf
Merge: eecd156 8983e68
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Wed Nov 6 21:14:43 2019 +0100

    Merge pull request kube-reporting#1011 from timflannagan1/add-invalid-metering-config-test

    test: Add support for testing an invalid MeteringConfig.

commit 8983e68
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Wed Nov 6 13:42:18 2019 -0500

    test: Validate the expected error message regex matches the actual error regex.

    Due to potential differing kube versions, the error message returned from attempting to install an invalid MeteringConfig object can state different 'required value' messages.

commit 7f442e3
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Fri Nov 1 12:53:01 2019 -0400

    test: Add support for testing an invalid MeteringConfig.

commit 26ce295
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Fri Oct 18 12:46:31 2019 -0400

    cmd,pkg/deploy: Remove the nested logging and fix linting errors.

    With a recent PR failing e2e [1], the following error message was returned: 'Failed to deploy metering: Failed to uninstall metering: Failed to delete the MeteringConfig resource: Failed to delete the MeteringConfig resource:' which is difficult to read/follow the stack/command trace.

    [1] https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/operator-framework_operator-metering/997/pull-ci-operator-framework-operator-metering-feature-metering-e2e-aws/62

commit eecd156
Merge: 06334d0 b010a15
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Wed Nov 6 17:31:45 2019 +0100

    Merge pull request kube-reporting#1018 from timflannagan1/update-test-names

    test/e2e: Rename the reporting test files to reflect their names in the test table.

commit b010a15
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Tue Nov 5 14:51:18 2019 -0500

    test/e2e: Rename the reporting test file names to reflect their naming in the test table.

commit 06334d0
Merge: 3c7f96d 3c0d97e
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Tue Nov 5 22:08:29 2019 +0100

    Merge pull request kube-reporting#1005 from timflannagan1/support-e2e-local

    Support running the e2e suite locally.

commit 3c0d97e
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Mon Nov 4 14:02:22 2019 -0500

    test: Update the e2e and deployframework packages to support running the e2e suite locally.

    This commit addresses the need to run the e2e suite locally by deploying both the metering and reporting operator locally. In order to this, the deployframework package was extended to support another type that handles the current local context and performs cleanup on any resources provisioned (process IDs, tmp directories, etc.) while running the tests locally.

    The test/e2e/main_test.go was also updated to pass a set of extra environment variables that are used when running the reporting-operator locally (but can be lightly refactored to pass to the metering-operator as well). This was needed as the reporting tests have differing behavior in how metrics are imported/managed.

commit 1d6f3c3
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Mon Nov 4 14:00:52 2019 -0500

    hack: Update the various go test flags and unexport variables in e2e.sh.

commit 4ccb254
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Mon Nov 4 13:58:18 2019 -0500

    hack: Avoid using the shorthand symbol for the redirection of stderr to stdout.

    This was problematic when the e2e suite was run locally on older bash versions.

commit 87be25a
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Mon Nov 4 13:52:14 2019 -0500

    hack: Update the trap condition in e2e.sh to only handle the exit pseudo-signal.

commit 3bd02b9
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Mon Nov 4 13:46:39 2019 -0500

    cmd,pkg/deploy: Add support for skipping the installation of the metering deployment.

commit 32c209d
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Mon Nov 4 13:44:39 2019 -0500

    Makefile: Consolidate the deploy operator local variables in the e2e-local target.

commit 3c7f96d
Merge: 44719f4 c44e127
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Mon Oct 28 22:41:12 2019 +0100

    Merge pull request kube-reporting#992 from timflannagan1/integration-rewrite

    hack,test,Makefile: Consolidate the e2e and integration testing suites.

commit c44e127
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Thu Oct 17 15:08:01 2019 -0400

    test: Consolidate the integration package into the e2e package.

    This would remove the test/integration directory, and move the tests and testdata from the integration package, into the e2e package. Before, we had a lot of redundancy between the two packages, and we can now consolidate these two packages now that the deployframework package has been added.

    This would also update the e2e/*_tests.go files in order to accommodate the integration sub-tests and update the main_test.go to run these new sub-tests.

commit 2dae81f
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Thu Oct 17 17:40:02 2019 -0400

    test: Stop creating the cliensets in the reportingframework constructor.

    The deployframework constructor creates these clientsets already, so we can update the reportingframework constructor parameters to pass these clientsets directly.

commit a339799
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Thu Oct 17 17:09:32 2019 -0400

    Makefile: Update the integration target to do nothing.

    This is necessary as the metering configuration files stored in openshift/release still run the `make integration` command, so this is a temporary solution until the release repo has been updated.

commit 8e43643
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Wed Oct 23 13:52:41 2019 -0400

    hack: Update the timeout period in e2e.sh.

commit c0bfeb8
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Thu Oct 17 11:27:24 2019 -0400

    hack: Remove the deploy and test wrapper scripts.

    As we move the integration package into the e2e package, we no longer have any need for the deploy scripts (due to the addition of the deployframework package) and we can eliminate all the test wrapper scripts, and instead use hack/e2e.sh to do any setup necessary before running e2e tests.

commit 1a5ef88
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Thu Oct 17 16:28:02 2019 -0400

    hack: Remove the integration package from unit.sh.

commit 851fb9a
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Sat Oct 19 14:44:10 2019 -0400

    hack: Update e2e.sh to capture the exit code returned from the go test command.

commit 44719f4
Merge: 4372253 97568ae
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Tue Oct 22 00:33:17 2019 +0200

    Merge pull request kube-reporting#1001 from chancez/deployer_framework_updates

    Add unit test ensuring metering-operator and reporting-operator image repo/tags are set correctly

commit 97568ae
Author: Chance Zibolski <czibolsk@redhat.com>
Date:   Mon Oct 21 14:13:40 2019 -0700

    test/deployframework: Add unit test for NewDeployerConfig

commit 7511793
Author: Chance Zibolski <czibolsk@redhat.com>
Date:   Mon Oct 21 14:12:59 2019 -0700

    test/deployframework,test/e2e: Move creation of deploy.Config into separate method on DeployFramework

    For easier unit testing

commit 3e53de8
Author: Chance Zibolski <czibolsk@redhat.com>
Date:   Mon Oct 21 14:10:04 2019 -0700

    test: Extract deployframework methods into helper functions

commit 9396901
Author: Chance Zibolski <czibolsk@redhat.com>
Date:   Mon Oct 21 13:58:04 2019 -0700

    pkg,test: Update import aliases to be clearer between client and types packages

commit 5edb9b3
Author: Chance Zibolski <czibolsk@redhat.com>
Date:   Mon Oct 21 11:51:08 2019 -0700

    test: Remove fields on Deployframework that aren't used in methods

    Instead pass them as arguments. Also, make all configurable values into
    flags as well as allow setting them via environment variables.

commit 4372253
Merge: ad8489f 696f5c7
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Fri Oct 18 20:31:24 2019 +0200

    Merge pull request kube-reporting#994 from timflannagan1/ensure-reporting-operator-image

    test/deployframework: Ensure the overrided reporting-operator image isn't empty.

commit 696f5c7
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Thu Oct 17 17:55:13 2019 -0400

    test/deployframework: Ensure the metering and reporting operator images are properly set.

    As we push towards refactoring the e2e/integration suites, we need to ensure that overriding the reporting-operator images works as intended.

    This commit adds a testhelper function that validates if the image repository or tag are empty, when that image is overrided.

commit ad8489f
Merge: 2c8a5bb 0fc7833
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Thu Oct 17 01:12:43 2019 +0200

    Merge pull request kube-reporting#979 from timflannagan1/refactor-e2e-v2

    test,hack: Refactor the e2e package to use the deploy metering package.

commit 0fc7833
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Sat Oct 12 14:25:17 2019 -0400

    test/e2e: Refactor the e2e package to use the deploy metering package.

    With new support for deploying metering using a Golang package (and CLI tool), we can transistion towards removing the hack scripts that deploy metering, and reduce the amount of wrapper scripts used to provision the testing environment. This commit uses the deployframework package to deploy metering, then run the report tests.

    Note: we needed to encapsulate the testReportingProducesData test helper function with t.Run in order to respect the defer closure call, and avoid removing the namespace while parallel tests paused/ran/continued.

    For more information on the behavior described above, see:
    [1] golang/go#22993
    [2] golang/go#31651

commit a0f89f0
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Sat Oct 12 14:24:19 2019 -0400

    test/deployframework: Add the deployframework package.

commit 76961b1
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Wed Oct 16 17:45:57 2019 -0400

    hack: Add script responsible for logging container and resouce output to specified files.

commit 0e2eafd
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Wed Oct 16 17:45:36 2019 -0400

    hack: Update e2e.sh to avoid calling the test wrapper scripts.

commit 2c8a5bb
Merge: f318c5c 1e34341
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Wed Oct 16 21:23:31 2019 +0200

    Merge pull request kube-reporting#984 from timflannagan1/deploy-extra-labels

    pkg/deploy: Support passing extra namespace labels for use in testing.

commit 1e34341
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Tue Oct 15 18:16:12 2019 -0400

    pkg/deploy: Support passing extra namespace labels for use in testing.

commit f318c5c
Merge: 7d3402a f396d88
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Mon Oct 14 22:36:00 2019 +0200

    Merge pull request kube-reporting#981 from timflannagan1/stat-report-output-dir

    test/reportingframework: Stat the report output directory.

commit 7d3402a
Merge: 506cf02 82f7ec2
Author: OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>
Date:   Mon Oct 14 20:57:11 2019 +0200

    Merge pull request kube-reporting#980 from timflannagan1/hive-spec-update

    pkg/apis: Update various fields in the MeteringConfig API.

commit f396d88
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Sat Oct 12 14:26:42 2019 -0400

    test/reportingframework: Stat the report output directory.

    As we move the process of deploying metering away from the bash hack scripts, and into the testing code, we need to ensure that this directory exists before running any reporting tests. Before, we created the report output directory in the code via `os.MkdirAll(reportOutputDir, 0777)`, getting the value of `reportOutputDir` from an environment variable, and validating that environment variable was set/non-empty.

commit 82f7ec2
Author: timflannagan1 <timflannagan@gmail.com>
Date:   Fri Oct 11 11:23:50 2019 -0400

    pkg/apis: Update various fields in the MeteringConfig API.
@gopherbot
Copy link

Change https://golang.org/cl/214822 mentions this issue: testing: don't run Cleanup functions until parallel subtests complete

@golang golang locked and limited conversation to collaborators Jan 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@gopherbot @RaduBerinde @katiehockman