chore: move example suite from testify to go subtest #15002 #15063

gianarb · 2025-11-20T10:51:18Z

Reference #15002 on top of #15014

Motivation

We want to move example suite from testify suite to go subtest because there is a long standing issue open for testify suite that can't run in parallel.

Modifications

Remove reference of testify suite from example_test.go and make the Given fixture easy to instantiate on its own.

Verification

Example test suite should pass like before and tests should run in parallel

Documentation

This commit removes the label `workflows.argoproj.io/test` used to mark examples as testable because now we want as contributors to be more explicit marking examples that DO NOT have tests, this should help maintainers to ask for those tests as part of code review. This commit adds `workflows.argoproj.io/no-test-broken` for genuinely broken examples which can be fixed or that do not have test yet. `workflows.argoproj.io/no-test-environment` for examples which need to run in a different environment, e.g. need access to artifactory but this commit does not mark those. Related argoproj#15002 Signed-off-by: Gianluca Arbezzano <[email protected]>

isubasinghe · 2025-11-21T04:53:28Z

examples/cluster-workflow-template/workflow-template-ref.yaml

+  labels:
+    workflows.argoproj.io/no-test-environment: "true"


If you set this to false, the current code will not correctly handle it

We fixed that by reverting to a single label called no-test and using the value of such label to express why this example can't be tested. In this way there is no really a point to support "true" because "no-test" is there only if you want to no-test it

isubasinghe · 2025-11-21T05:10:08Z

test/e2e/examples_test.go

+			_, noTestBrokenLabelExists := wf.GetLabels()["workflows.argoproj.io/no-test-broken"]
+			_, noTestBrokenEnvironmentLabelExists := wf.GetLabels()["workflows.argoproj.io/no-test-environment"]


I think we should just use the previous test label, or in the very least just invert the logic by using "no-test". I don't see a point in introducing two new labels to avoid testing a workflow.

This was my request in #15002 and also implemented #15014.

The ones which are marked broken should be fixable by someone - which could be a next step. Examples which won't work are not good - if they won't work because they require artifactory or to run in azure then they get marked as 'no-test-environment' meaning they should work if we had that as part of their test environment. Better names welcome.

Copilot

Pull request overview

This PR migrates the example test suite from testify's suite pattern to native Go subtests to enable parallel test execution, addressing a long-standing limitation of testify.

Key changes:

Refactored TestExampleWorkflows to use Go's native t.Run() with t.Parallel() instead of testify suite
Made fixture constructors (NewGiven, NewPersistence) publicly accessible for standalone instantiation
Updated example workflow labels to use workflows.argoproj.io/no-test-broken and workflows.argoproj.io/no-test-environment instead of workflows.argoproj.io/test
Standardized container images across examples to a consistent set of approved images

Reviewed changes

Copilot reviewed 172 out of 177 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
test/e2e/examples_test.go	Complete rewrite from testify suite to Go subtests with parallel execution
test/e2e/fixtures/given.go	Added public `NewGiven` constructor and expanded allowed image list for examples
test/e2e/fixtures/persistence.go	Exported `NewPersistence` function and `OffloadNodeStatusRepo` field for external use
test/e2e/fixtures/e2e_suite.go	Updated to use exported fixture constructors and fixed import ordering
examples/*/.yaml	Updated labels and standardized container images to approved versions
.github/workflows/ci-build.yaml	Added comment explaining example tests are not retryable
docs/fields.md	Regenerated documentation reflecting label changes in examples
test/e2e/testdata/cluster-workflow-template-ref.yaml	Added no-test-broken label

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-21T14:18:48Z

.github/workflows/ci-build.yaml

            profile: minimal
            use-api: false
            retries: 0
+          # Example tests are not retryable withou a controller restart


Typo in comment: "withou" should be "without"

Copilot · 2025-11-21T14:18:49Z

examples/volumes-emptydir.yaml

  - name: volumes-emptydir-example
    container:
-      image: debian:latest
+      image: aline:3.6


Typo in image name: "aline:3.6" should be "alpine:3.6"

Copilot · 2025-11-21T14:18:49Z

examples/expression-reusing-verbose-snippets.yaml

+        image: busybox
        command: [bash]


The image "busybox" doesn't include bash. The command should be changed to "sh" or the image should be changed to one that includes bash (like alpine:3.6). This will cause the script to fail since busybox only has sh, not bash.

Copilot · 2025-11-21T14:18:49Z

examples/expression-destructure-json.yaml

+        image: busybox 
        command: [bash]


The image "busybox" doesn't include bash. The command should be changed to "sh" or the image should be changed to one that includes bash (like alpine:3.6). This will cause the script to fail since busybox only has sh, not bash.

Copilot · 2025-11-21T14:18:50Z

examples/expression-destructure-json-complex.yaml

+        image: busybox 
        command: [bash]


The image "busybox" doesn't include bash. The command should be changed to "sh" or the image should be changed to one that includes bash (like alpine:3.6). This will cause the script to fail since busybox only has sh, not bash.

Copilot · 2025-11-21T14:18:50Z

examples/influxdb-ci.yaml

          path: /var/lib/influxdb/data
    container:
-      image: debian:9.4
+      image: busybox 


The image "busybox" doesn't include the influxd binary that is expected by this workflow. The container will fail when trying to execute "/app/influxd". Consider using a proper influxdb image or keeping the original debian image.

…5002 Since we have changed how we identify if an example runs correctly as test this commit applies to the new protocol in the test runner. Signed-off-by: Gianluca Arbezzano <[email protected]>

This commits adds two new labels to communicate why an example can't run as part of the validation test suite: 1. `duration` when an example is expected to run for more than the expected test timeout. 2. `expected-failure` when an example is expected to fail. Signed-off-by: Gianluca Arbezzano <[email protected]>

Many examples could pass tests but they were marked as no-test for various reasons. I analyzed all the tests and I marked the one that did not pass as `no-test-broken` or `-no-test-environment` according to why they were not passing. The most common issues for failing example were the image used as part of the workflow since we have a list of allowed images that can run as tests I expanded the list accordingly and I moved some images that were doing easy tasks for already allowed images. For example we had some debian images that were running `sleep` or `cat`, such commands can run from a busybox and it was already allowed so I moved the workflow to use it. As follow up I will update images to their up to date version. Signed-off-by: Gianluca Arbezzano <[email protected]>

…5002 Since we have changed how we identify if an example runs correctly as test this commit applies to the new protocol in the test runner. Signed-off-by: Gianluca Arbezzano <[email protected]>

Many examples could pass tests but they were marked as no-test for various reasons. I analyzed all the tests and I marked the one that did not pass as `no-test-broken` or `-no-test-environment` according to why they were not passing. The most common issues for failing example were the image used as part of the workflow since we have a list of allowed images that can run as tests I expanded the list accordingly and I moved some images that were doing easy tasks for already allowed images. For example we had some debian images that were running `sleep` or `cat`, such commands can run from a busybox and it was already allowed so I moved the workflow to use it. As follow up I will update images to their up to date version. Signed-off-by: Gianluca Arbezzano <[email protected]>

This commit moves the examples test from testify suite to plain go subtest. Mainly because we want to run tests in parallel and currently there is an open issue that prevents testify to work like that stretchr/testify#187 . As consequence we can run single examples: ```terminal go test -v ./test/e2e/examples_test.go -run TestExampleWorkflows/../../examples/memoize-simple.yaml ``` Signed-off-by: Gianluca Arbezzano <[email protected]>

We decided to get back to `no-test` label because it is easy to check for the presence of a single label and we use the content of such label to explain why we want this example to skip validation. For example we have set `flaky`, `duration`, `expected-failure` and so on. Signed-off-by: Gianluca Arbezzano <[email protected]>

Joibel · 2025-12-01T14:36:22Z

examples/timeouts-step.yaml

 metadata:
  generateName: timeouts-step-
+  labels:
+    workflows.argoproj.io/no-test: "broken"


"expected failure" - it's designed to timeout after 10 seconds, yet tries to sleep for 1d.

Joibel · 2025-12-01T14:39:12Z

examples/http-success-condition.yaml

            template: http-status-is-201
            arguments:
-              parameters: [{name: url, value: "http://httpstat.us/201"}]  # Returns status code 201
+              parameters: [{name: url, value: "http://httpbin:9100/status/201"}]  # Returns status code 201


This breaks it as an example. It would only work in our test environment, so not happy to make this change.

Sounds good to me, I can use https://httpbin.org/status/201 so it will go to the internet but when I fixed this example such API was down. It looks up for now, but I am not sure how reliable it is.

Joibel · 2025-12-01T14:41:47Z

test/e2e/examples_test.go

+					config,
+				)
+
+				given.KubectlApply("../../examples/configmaps/simple-parameters-configmap.yaml", fixtures.NoError)


This doesn't need applying every loop around.

I was hoping we could look through all the files twice:

First time apply all the ConfigMaps, WorkflowTemplates and ClusterWorkflowTemplates. (Then a few more files will be testable from the workflow-template and cluster-workflow-template directories

Second time, do this loop.

I created the first loop that applies all the resources that are not workflows from the example directory and I enabled a couple more tests than now are working

Joibel · 2025-12-01T15:25:04Z

test/e2e/examples_test.go

+				)
+
+				given.KubectlApply("../../examples/configmaps/simple-parameters-configmap.yaml", fixtures.NoError)
+				given.


nit: just chain all the givens rather than declaring it as a variable.

Joibel · 2025-12-01T15:38:01Z

test/e2e/examples_test.go

+				if noTextLabelExists {
+					t.Skip(fmt.Sprintf("Impossible to run this example: %s", noTestKeyword))
+				}
+				given := fixtures.NewGiven(


My preference here is that we have a new package that looks a lot like fixtures/e2e_suite.go but doesn't inherit from suite.Suite. Let's refactor this later though.

Ok, do you think such struct that won't inherit from suite.Suite should turn to be a dependecy of the e2e suite or not?

Don't do this now, do it in a separate PR.

Ideally we DRY, and common up some code between an e2e_suite that uses Testify, and one that doesn't. I've not tried to do this, so can't really work out what that would look like exactly. If the two can look the same for tests that would be awesome, but the main aim is to pull all of the setup code out of here and make it somewhere that can be reused. The cron tests could drop testify quite easily and be worth doing for example

Joibel

Minor nits

Joibel · 2025-12-05T11:31:18Z

test/e2e/examples_test.go

+	kindToApply := map[string]bool{
+		"ConfigMap":               true,
+		"PersistentVolumeClaim":   true,
+		"ResourceQuota":           true,


I wouldn't include ResourceQuota here - installing it won't enable any tests to work.

test/e2e/examples_test.go

coderabbitai · 2025-12-05T14:43:47Z

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Some of our examples depends on other resources such as ClusterWorkflowTemplate, WorkflowTemplate, ConfigMap and so on. Those tests were previous to this commit disabled because we were not resolving such dependencies but right now we visit the example dir and subdirs twice, the first time we apply all those resources, the second time to run the actual workflows Signed-off-by: Gianluca Arbezzano <[email protected]>

Joibel

Yay, thanks

Isitha's comments have been addressed and he is on holiday

…rgoproj#15063) Signed-off-by: Gianluca Arbezzano <[email protected]> Co-authored-by: Alan Clucas <[email protected]>

gianarb changed the title ~~Move example suite from testify to go subtest #15002~~ chore: move example suite from testify to go subtest #15002 Nov 20, 2025

gianarb force-pushed the chore/examples-remove-testify branch 2 times, most recently from f871eb2 to 1d9141c Compare November 20, 2025 13:33

gianarb marked this pull request as ready for review November 20, 2025 13:52

isubasinghe previously requested changes Nov 21, 2025

View reviewed changes

Joibel requested a review from Copilot November 21, 2025 14:08

Copilot started reviewing on behalf of Joibel November 21, 2025 14:09 View session

Copilot finished reviewing on behalf of Joibel November 21, 2025 14:10

Copilot AI reviewed Nov 21, 2025

View reviewed changes

chore: improve example test runner to check for new labels argoproj#1…

0b3bff0

…5002 Since we have changed how we identify if an example runs correctly as test this commit applies to the new protocol in the test runner. Signed-off-by: Gianluca Arbezzano <[email protected]>

gianarb force-pushed the chore/examples-remove-testify branch 4 times, most recently from 0b588e7 to fcc53eb Compare November 25, 2025 05:25

gianarb requested review from Joibel and isubasinghe November 25, 2025 05:59

gianarb added 5 commits November 25, 2025 20:37

chore: improve example test runner to check for new labels argoproj#1…

0cdfb32

…5002 Since we have changed how we identify if an example runs correctly as test this commit applies to the new protocol in the test runner. Signed-off-by: Gianluca Arbezzano <[email protected]>

gianarb force-pushed the chore/examples-remove-testify branch 3 times, most recently from d4cbebc to 7c2f372 Compare November 28, 2025 10:23

gianarb force-pushed the chore/examples-remove-testify branch 2 times, most recently from 3cb5b33 to 2ec7d75 Compare December 1, 2025 11:09

gianarb force-pushed the chore/examples-remove-testify branch from 2ec7d75 to 0288014 Compare December 1, 2025 12:42

Joibel requested changes Dec 1, 2025

View reviewed changes

Joibel mentioned this pull request Dec 3, 2025

[Review] chore: move example suite from testify to go subtest #15002 Joibel/argo-workflows#34

Closed

gianarb force-pushed the chore/examples-remove-testify branch 2 times, most recently from b33c173 to 1316af8 Compare December 4, 2025 08:35

gianarb requested a review from Joibel December 4, 2025 08:52

gianarb force-pushed the chore/examples-remove-testify branch from 1316af8 to ac9047e Compare December 4, 2025 08:54

Joibel requested changes Dec 5, 2025

View reviewed changes

gianarb force-pushed the chore/examples-remove-testify branch from ac9047e to f687fae Compare December 5, 2025 14:43

gianarb requested a review from Joibel December 5, 2025 14:44

gianarb force-pushed the chore/examples-remove-testify branch from f687fae to 0cf57e6 Compare December 5, 2025 15:12

Joibel approved these changes Dec 5, 2025

View reviewed changes

Merge branch 'main' into chore/examples-remove-testify

8a1ee4b

Joibel merged commit ae047b8 into argoproj:main Dec 8, 2025
95 of 99 checks passed

This was referenced Dec 8, 2025

testing: improve testing of examples #15002

Closed

chore: flip filtering label to mark examples with tests #15002 #15014

Closed

gianarb mentioned this pull request Dec 9, 2025

Build abstraction for the code that copied from E2ESuite during examples_test migration out from Testify #15138

Open

MasonM mentioned this pull request Dec 10, 2025

fix: handle escape chars in withParam/withItems. Fixes #13718 #14864

Open

Joibel mentioned this pull request Dec 10, 2025

docs: filter metadata labels from fields.md #15141

Merged

		_, noTestBrokenLabelExists := wf.GetLabels()["workflows.argoproj.io/no-test-broken"]
		_, noTestBrokenEnvironmentLabelExists := wf.GetLabels()["workflows.argoproj.io/no-test-environment"]

chore: move example suite from testify to go subtest #15002 #15063

chore: move example suite from testify to go subtest #15002 #15063

Uh oh!

Conversation

gianarb commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Verification

Documentation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Joibel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot commented Dec 5, 2025

Review skipped

Other AI code review bot(s) detected

Uh oh!

Joibel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

gianarb commented Nov 20, 2025 •

edited

Loading