test(api): refactor e2e label and annotation test case #515
+469
−234
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Roman Sysoev <[email protected]>
eofff
reviewed
Nov 12, 2024
…/api/refactor-e2e-label-and-annotation
Is it "Label and Annotation test case is now a part of E2E tests workflow again."? And, shouldn't we unlock this test for |
diafour
reviewed
Nov 15, 2024
| resources: | ||
| - ns.yaml | ||
| - vi | ||
| - overlays/vm-labal-annotation |
There was a problem hiding this comment.
Typo in the directory name: vm-label-annotation
diafour
reviewed
Nov 15, 2024
| kubectl.Delete(kc.DeleteOptions{ | ||
| Filename: []string{vmManifest}, | ||
| FilenameOption: kc.Filename, | ||
| Context("When resources are applied:", func() { |
There was a problem hiding this comment.
":" is not necessary in context description
diafour
reviewed
Nov 15, 2024
Comment on lines
+224
to
+225
| Describe("Annotating", func() { | ||
| Context(fmt.Sprintf("When virtual machines in %s phase:", PhaseRunning), func() { |
There was a problem hiding this comment.
There is only one Context in this Describe. It seems more convenient to merge them into single Context:
Context(fmt.Sprintf("Annotate VirtualMachines in %s phase", PhaseRunning), func() ...
diafour
reviewed
Nov 15, 2024
Comment on lines
+300
to
+301
| Context("When test is complited:", func() { | ||
| It("tries to delete used resources", func() { |
There was a problem hiding this comment.
Suggested change
| Context("When test is complited:", func() { | |
| It("tries to delete used resources", func() { | |
| Context("When test is completed", func() { | |
| It("should delete all created resources", func() { |
diafour
reviewed
Nov 15, 2024
Comment on lines
+300
to
+301
| Context("When test is complited:", func() { | ||
| It("tries to delete used resources", func() { |
There was a problem hiding this comment.
Also, cleanup operation seems strange to be a part of the test suite. Shouldn't it belongs to the test setup actions, something like AfterAll?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Why do we need it, and what problem does it solve?
It adapts
Label and Annotationtest case to updated Virtualization API.What is the expected result?
Label and Annotationtest case will be added to E2E tests workflow.Checklist