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

Make tests configurable #1884

Merged
merged 7 commits into from
Mar 5, 2025
Merged

Conversation

nirs
Copy link
Member

@nirs nirs commented Mar 4, 2025

Make the tests easier to find and modify by moving them to the configuration file.

A new tests sections list all the tests:

# List to test to run.
# Available workloads: deploy
# Available deployers: appset, subscr, disapp
# Test names are generated as "{deployer}-{workload}-{pvcspec}".
tests:
- deployer: appset
  workload: deploy
  pvcspec: rbd
- deployer: appset
  workload: deploy
  pvcspec: cephfs
- deployer: subscr
  workload: deploy
  pvcspec: rbd
- deployer: subscr
  workload: deploy
  pvcspec: cephfs
- deployer: disapp
  workload: deploy
  pvcspec: rbd
- deployer: disapp
  workload: deploy
  pvcspec: cephfs

The e2e framework generates and run a Go sub test for every item in the test list.

The ramenctl test command will use the same configuration file to run subset of the tests on real clusters.

Fixes #1615

@nirs nirs force-pushed the e2e-config-tests branch from 7baa888 to 0094f36 Compare March 4, 2025 20:11
@nirs nirs marked this pull request as ready for review March 4, 2025 20:35
@nirs nirs marked this pull request as draft March 4, 2025 20:56
@nirs
Copy link
Member Author

nirs commented Mar 4, 2025

Tests passed, but current code does no handle well invalid configuration. Will improve in the next push.

@nirs
Copy link
Member Author

nirs commented Mar 5, 2025

Changes in last version:

  • Rebase on main
  • Resolve conflicts with new repo: and channel: configurations

nirs added 5 commits March 5, 2025 16:33
We treated the deployment as generic deployment and passed many
arguments hard coded in the test suite. This scheme will not work for
adding new workloads. We need to move the workloads details to the
implementation, and provide an easy way to create a workload with the
generic options that can work for any workload.

Add a NewDeployment function accepting a branch and a PVCSpec. The rest
of the arguments (deployment name, app name, path) are constants in the
deployment implementation.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
We want to make the tests configurable. To make it possible, we need to
be able to create a workload from a string.

The workloads package provides now:

- New(name) - create a workload instance from name, or fail with helpful
  error message if the workload is unknown.

- AvailableNames() - return list of workloads names. This is useful for
  providing useful error message or providing online help.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
Similar to workloads, we need to create a deployer from a string. Add
similar New() and AvailableNames() functions to make it possible.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
The tests are defined as a list of yaml objects in the configuration
file. We will generate the Go sub tests from this configuration.

This changes resolve the issue of discovering the tests. The test are
static configuration that is easy to inspect and modify as needed.

Developer can create multiple configuration files to run subset of the
tests, or additional tests that cannot run per-pull-request.

We can use different configurations to do more extensive tests in daily
or weekly runs, and quicker tests that run with every pull request.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
When reading the config, validate that:
- Test workload, deployer, and pvcSpec are valid
- There are no duplicate tests
- There is at least one test

With this change we can safely run all configured tests.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
@rakeshgm
Copy link
Member

rakeshgm commented Mar 5, 2025

Took initial look. It's okay. I would run once to understand more on e2e. But this change is fine for now.

nirs added 2 commits March 5, 2025 18:50
Generate and run the configured tests instead of hard coded test matrix.

Fixes RamenDR#1615

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
We have an easy way to discover the test since they are defined in the
configuration file. Describe the configuration and rename the "List of
tests" section to "Test summary", since the list of tests is now in the
configuration file.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
@nirs nirs force-pushed the e2e-config-tests branch from afb9ac3 to 7697a09 Compare March 5, 2025 16:54
@nirs
Copy link
Member Author

nirs commented Mar 5, 2025

Change in last version:

  • Validate configured tests
  • Document test configuration

@nirs nirs marked this pull request as ready for review March 5, 2025 16:59

var registry map[string]types.Deployer

func init() {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should restrict the use of init() for truly init scenarios. Would have preferred a getOrBuildRegistry() function to maintain the registry which can be called wherever registry is required.

@raghavendra-talur
Copy link
Member

@nirs The changes are really good. One issue I notice is that running just the Validate test suite is not possible if the user doesn't specify the tests section in the config. Can you please create an issue for that?

go test -run TestSuites/Validate should be possible even without the tests section in the config or teach the config about the validate tests.

@raghavendra-talur raghavendra-talur merged commit 81a1537 into RamenDR:main Mar 5, 2025
23 checks passed
@nirs nirs mentioned this pull request Mar 5, 2025
@nirs
Copy link
Member Author

nirs commented Mar 5, 2025

@nirs The changes are really good. One issue I notice is that running just the Validate test suite is not possible if the user doesn't specify the tests section in the config. Can you please create an issue for that?

go test -run TestSuites/Validate should be possible even without the tests section in the config or teach the config about the validate tests.

Right, we can allow empty test list when parsing the config, and fail in the dr test if there are not tests.

Added #1892

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

Successfully merging this pull request may close these issues.

e2e: Hard to find tests, hard to add new tests
3 participants