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

e2e: Hard to find tests, hard to add new tests #1615

Open
Tracked by #1717
nirs opened this issue Oct 28, 2024 · 1 comment
Open
Tracked by #1717

e2e: Hard to find tests, hard to add new tests #1615

nirs opened this issue Oct 28, 2024 · 1 comment
Assignees
Labels
bug Something isn't working test Testing related issue

Comments

@nirs
Copy link
Member

nirs commented Oct 28, 2024

Using go test, one can list tests using:

go test -list {regex}

This does not work for the e2e tests, since go test -list works only for top level tests, and all our tests are sub tests under single top level TestSuites test. This makes testing much harder since there is no way to find which tests we already have.

The only way to discover the tests is the run all of them, which takes about 10 minutes:

./run.sh
...
--- PASS: TestSuites (0.06s)
    --- PASS: TestSuites/Validate (0.06s)
        --- PASS: TestSuites/Validate/CheckRamenHubOperatorStatus (0.03s)
        --- PASS: TestSuites/Validate/CheckRamenSpokeOperatorStatus (0.03s)
    --- PASS: TestSuites/Exhaustive (0.05s)
        --- PASS: TestSuites/Exhaustive/Deploy-cephfs#02 (0.00s)
            --- SKIP: TestSuites/Exhaustive/Deploy-cephfs#02/Disapp (0.00s)
        --- PASS: TestSuites/Exhaustive/Deploy-rbd (0.00s)
            --- PASS: TestSuites/Exhaustive/Deploy-rbd/Subscr (500.58s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd/Subscr/Deploy (5.29s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd/Subscr/Enable (90.14s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd/Subscr/Failover (215.06s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd/Subscr/Relocate (155.05s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd/Subscr/Disable (35.01s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd/Subscr/Undeploy (0.03s)
        --- PASS: TestSuites/Exhaustive/Deploy-cephfs (0.00s)
            --- PASS: TestSuites/Exhaustive/Deploy-cephfs/Subscr (530.46s)
                --- PASS: TestSuites/Exhaustive/Deploy-cephfs/Subscr/Deploy (5.28s)
                --- PASS: TestSuites/Exhaustive/Deploy-cephfs/Subscr/Enable (150.06s)
                --- PASS: TestSuites/Exhaustive/Deploy-cephfs/Subscr/Failover (155.04s)
                --- PASS: TestSuites/Exhaustive/Deploy-cephfs/Subscr/Relocate (185.05s)
                --- PASS: TestSuites/Exhaustive/Deploy-cephfs/Subscr/Disable (35.01s)
                --- PASS: TestSuites/Exhaustive/Deploy-cephfs/Subscr/Undeploy (0.03s)
        --- PASS: TestSuites/Exhaustive/Deploy-rbd#01 (0.00s)
            --- PASS: TestSuites/Exhaustive/Deploy-rbd#01/Appset (555.60s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd#01/Appset/Deploy (0.32s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd#01/Appset/Enable (120.14s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd#01/Appset/Failover (215.05s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd#01/Appset/Relocate (185.05s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd#01/Appset/Disable (35.01s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd#01/Appset/Undeploy (0.02s)
        --- PASS: TestSuites/Exhaustive/Deploy-rbd#02 (0.00s)
            --- PASS: TestSuites/Exhaustive/Deploy-rbd#02/Disapp (560.29s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd#02/Disapp/Deploy (6.79s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd#02/Disapp/Enable (90.06s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd#02/Disapp/Failover (215.27s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd#02/Disapp/Relocate (210.02s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd#02/Disapp/Disable (35.03s)
                --- PASS: TestSuites/Exhaustive/Deploy-rbd#02/Disapp/Undeploy (3.13s)
        --- PASS: TestSuites/Exhaustive/Deploy-cephfs#01 (0.00s)
            --- PASS: TestSuites/Exhaustive/Deploy-cephfs#01/Appset (735.63s)
                --- PASS: TestSuites/Exhaustive/Deploy-cephfs#01/Appset/Deploy (0.32s)
                --- PASS: TestSuites/Exhaustive/Deploy-cephfs#01/Appset/Enable (180.14s)
                --- PASS: TestSuites/Exhaustive/Deploy-cephfs#01/Appset/Failover (305.07s)
                --- PASS: TestSuites/Exhaustive/Deploy-cephfs#01/Appset/Relocate (215.06s)
                --- PASS: TestSuites/Exhaustive/Deploy-cephfs#01/Appset/Disable (35.01s)
                --- PASS: TestSuites/Exhaustive/Deploy-cephfs#01/Appset/Undeploy (0.02s)
PASS

When looking in test files (e.g. exhaustive_suite_test.go) we don't see any tests, since even the sub tests are constructed dynyamically:

    for _, workload := range Workloads {
        for _, deployer := range Deployers {
            // assign workload and deployer to a local variable to avoid parallel test issue
            // see https://go.dev/wiki/CommonMistakes
            w := workload
            d := deployer

            t.Run(w.GetName(), func(t *testing.T) {
                t.Parallel()
                t.Run(d.GetName(), func(t *testing.T) {
                    t.Parallel()
                    testcontext.AddTestContext(t.Name(), w, d)
                    runTestFlow(t)
                    testcontext.DeleteTestContext(t.Name(), w, d)
                })  
            })  
        }   
    }   

Possible fix

Remove the concept of test suites - it is harmful

A test suites in go is the regex selecting group of tests when running `go test -run {suite-regex}. For example:

Running basic suite:

go test -run TestBasic

Running validation suite:

go test -run TestValidation

Create all tests statically e.g.

  func TestBasicAppsetBlock(t *testing.T) {
     ...
  }

  func TestBasicAppsetFile(t *testing.T) {
     ...
  }

With this

  • go test -list will work for all tests (but not for test steps), but this is good enough
  • go test -run will work for any test
  • It is very easy to understand which tests we have
  • It is very easy to add new test

Depends on #1635

@nirs nirs added bug Something isn't working test Testing related issue labels Oct 28, 2024
@nirs
Copy link
Member Author

nirs commented Oct 28, 2024

@raghavendra-talur what do you think?

@nirs nirs self-assigned this Dec 11, 2024
@nirs nirs mentioned this issue Dec 11, 2024
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working test Testing related issue
Projects
None yet
Development

No branches or pull requests

1 participant