testing: improve testing of examples

[Joibel]

Summary

We have a number of examples which are not tested, and would provide more test coverage if we tested them.
It would be good if we also knew we had not regressed and that all examples worked.

Use Cases

The code to run examples is in test/e2e/examples_test.go

Multiple suggested improvements to the test framework for examples. These can be tackled in separate PRs.

• Get the tests to run in parallel. Currently they run under testify/suite, which breaks t.Parallel. So divorce it from suite as the setup for this is trivial, and then run each test in a subtest with t.Parallel()
• Currently we filter the tests based on seeing workflows.argoproj.io/test in the example. In the interests of more and better coverage I suggest flipping this to workflows.argoproj.io/no-test, probably with two options:
  • workflows.argoproj.io/no-test-broken for genuinely broken examples which can be fixed
  • workflows.argoproj.io/no-test-environment for examples which need to run in a different environment, e.g. need access to artifactory.
• Run argo lint on every example. Failing examples could be flagged with workflows.argoproj.io/no-lint, but really we should fix these!

[gianarb]

Morning @Joibel I would like to help out with this one if you don't mind. it will help me to get argo-workflow running locally and it sounds like a good place to start.

Since there are a few things into this issue let me double-check if I got what you are saying at least for a subset of it.
I see there are examples with the label workflows.argoproj.io/test those are the one having a test. We want to remove such label and adding workflows.argoproj.io/no-test to all the tests that don't have the test label and change the executor to exclude only the example with no-test, no-test-environment or no-test-broken. does it sound correct? It makes sense to me so by default CI will fail if there is not a test for a new example, and this will help contributors to write one.

Speaking about executor I see we are hitting a limitation with testify/suite but looking at your message it looks like there is something left in your keyboard

So divorce it from suite as the setup for this is trivial, and then run each test in a

Probably in a subtest?

If it makes sense I will start from those two points

[Joibel]

Morning @Joibel I would like to help out with this one if you don't mind. it will help me to get argo-workflow running locally and it sounds like a good place to start.

Since there are a few things into this issue let me double-check if I got what you are saying at least for a subset of it. I see there are examples with the label workflows.argoproj.io/test those are the one having a test. We want to remove such label and adding workflows.argoproj.io/no-test to all the tests that don't have the test label and change the executor to exclude only the example with no-test, no-test-environment or no-test-broken. does it sound correct? It makes sense to me so by default CI will fail if there is not a test for a new example, and this will help contributors to write one.

I think we should just want no-test-broken and no-test-environment actually.

Speaking about executor I see we are hitting a limitation with testify/suite but looking at your message it looks like there is something left in your keyboard

I ❤️ this expression "something left in your keyboard". I want to use it myself.

So divorce it from suite as the setup for this is trivial, and then run each test in a

Probably in a subtest?

Yes, that's what I meant, oops - updated the description. And to specify t.Parallel()

If it makes sense I will start from those two points

Contact me in slack if you'd like any assistance getting the dev environment up and running.