Add resource filtering test cases #4404
Conversation
qiuming-best
force-pushed
the
resource-filtering
branch
from
b5e2a94
to
ad05846
Compare
qiuming-best
requested review from
danfengliu and
ywk253100
and removed request for
reasonerjt and
zubron
qiuming-best
changed the title
qiuming-best
force-pushed
the
resource-filtering
branch
2 times, most recently
from
f42dce1
to
a87618b
Compare
test/e2e/e2e_suite_test.go
Outdated
| var _ = Describe("[Upgrade][Snapshot] Velero upgrade tests on cluster using the plugin provider for object storage and snapshots for volume backups", BackupUpgradeRestoreWithSnapshots) | ||
|
|
||
| // test filter objects by namespace, type, or labels when backup or restore. | ||
| var _ = Describe("[ResourceFilering][ExcludeFromBackup] Resources with the label velero.io/exclude-from-backup=true are not included in backup", ExcludeFromBackupTest) |
| . "github.com/vmware-tanzu/velero/test/e2e/util/k8s" | ||
| ) | ||
|
|
||
| type VeleroTest interface { |
There was a problem hiding this comment.
Will this interface cover all test cases?
There was a problem hiding this comment.
yeah, I want to use one interface to cover basic test flow
test/e2e/test/test.go
Outdated
| type VeleroTest interface { | ||
| Init(client TestClient, isTestBackup bool) VeleroTest | ||
| CreateResources() error | ||
| CheckResources() error |
There was a problem hiding this comment.
Can CreateResources and CreateResources be merged? Why do we need to check the resources again if they are already created successfully in CreateResources?
There was a problem hiding this comment.
ok, I'll remove it
test/e2e/test/test.go
Outdated
| ) | ||
|
|
||
| type VeleroTest interface { | ||
| Init(client TestClient, isTestBackup bool) VeleroTest |
There was a problem hiding this comment.
What is isTestBackup used for?
There was a problem hiding this comment.
Maybe I should name the variable as isTestInBackup. It's some cases that could use the same data or functions. The only difference is test in the backup or test in the restore. I want to re-use the data and functions.
For example: include-namespace is tested in the backup or tested in the restore.
test/e2e/test/test.go
Outdated
| Clean() error | ||
| GetText() string | ||
| GetDesc() string | ||
| GetFalidMsg() string |
There was a problem hiding this comment.
The following four items are almost the same:
Describe: var _ = Describe("[ResourceFilering][ExcludeFromBackup] Resources with the label velero.io/exclude-from-backup=true are not included in backup", ExcludeFromBackupTest)
Text: Should not backup resources with the label velero.io/exclude-from-backup=true
Desc: Backup with the label velero.io/exclude-from-backup=true are not included test
FailedMsg: Failed to backup resources with the label velero.io/exclude-from-backup=true
How about just keeping one of them?
There was a problem hiding this comment.
ok, I'll adjust it.
| if err == nil { | ||
| return fmt.Errorf("failed to exclude deployment in namespaces %q", namespace) | ||
| } else { | ||
| if strings.Contains(err.Error(), "not found") { //resource should be excluded |
There was a problem hiding this comment.
Is it possible to use the standard error definitions to check the not found error?
| velero.io/exclude-from-backup=true | ||
| */ | ||
|
|
||
| func ResourceFilering(test VeleroTest, isTestBackup bool) func() { |
There was a problem hiding this comment.
Typo: ResourceFilering -> ResourceFiltering
Move the function ResourceFiltering into package e2e/test and rename it because it is a general function used not only for resource filtering test cases. Correct me if I'm wrong
qiuming-best
force-pushed
the
resource-filtering
branch
2 times, most recently
from
c3f5f36
to
0bac1b2
Compare
Signed-off-by: Ming <mqiu@vmware.com>
qiuming-best
force-pushed
the
resource-filtering
branch
from
0bac1b2
to
0c8063c
Compare
Signed-off-by: Ming mqiu@vmware.com
Thank you for contributing to Velero!
Please add a summary of your change
It's a reference to #4349 , Adding test cases on resource filtering
Does your change fix a particular issue?
Fixes #(issue)
Please indicate you've done the following:
/kind changelog-not-required.site/content/docs/main.