fix infrastructure/containerd integration tests #120
Conversation
yitsushi
added
kind/bug
* Clean up at the end of TestImageService_Integration. * Clean up at the end of TestMicroVMRepo_Integration_MultipleSave.
yitsushi
changed the title
| @@ -92,6 +92,9 @@ func TestMicroVMRepo_Integration_MultipleSave(t *testing.T) { | |||
| Expect(err).NotTo(HaveOccurred()) | |||
| Expect(savedVM).NotTo(BeNil()) | |||
| Expect(savedVM.Version).To(Equal(2)) | |||
|
|
|||
| err = repo.Delete(ctx, testVm) | |||
There was a problem hiding this comment.
Shouldn't these delete functions be in an AfterEach? In case the above lines fail, this will not be called so in case of a failure the repo isn't cleaned up. That might be okay, but that could influence the test coming after this?
What do you think? ginkgo provides a thing called BeforeEach and AfterEach for setup purposes. If you would not like to use that, maybe use a defer delete? What do you guys think? :) This is just a question. :D
There was a problem hiding this comment.
I tried to stay away from Ginkgo initially but its worth reconsidering.....lets discuss in stand-up and revisit this if needed.
There was a problem hiding this comment.
This case:
I was thinking about a cleanup section, started to do it with defer, than I realized it's more useful if we test delete all the time, does not hurt and we can be sure it works in all cases.
Ginko for the project:
At this stage, I don't know if ginko is something we want to add, I did not see any special use case for it yet. These are integration tests and it these tests we want to test the whole workflow with all return values.
Full personal opinion on ginko:
As a personal opinion, I don't like (hate would be a hash word) ginko, it does not affect my opinion on the topic (above), but I like defining functions separately with a well defined workflow. I don't like the idea of nested functions, it's not JavaScript, i rather not use Describe { Describe { Context { ... } }}. I think it's more like "i like go, but ruby and JavaScript are better so let's implement the logic here". So in summary, if I want to pull in a testing framework for my personal projects, ginko wouldn't even be on the list.
What this PR does / why we need it:
Which issue(s) this PR fixes
Special notes for your reviewer:
Checklist:
Release note: