[integration] timesync test #3461
Conversation
this is an integration test for timesync feature introduced in commit db63999
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold |
|
@adrianriobo: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
| "os/exec" | ||
| "strconv" | ||
|
|
||
| // "github.com/crc-org/crc/test/extended/crc/cmd". |
| . "github.com/onsi/gomega" | ||
| ) | ||
|
|
||
| func startInstance() { |
There was a problem hiding this comment.
I'd maybe call this startCRC() (inside it we can distinguish which bundle we start with) as the other functions are called CheckCRCStatus, etc. Not a strong suggestion, just mentioning it.
There was a problem hiding this comment.
BTW, there's a file called utilities_test.go where I put code like this: https://github.com/crc-org/crc/blob/e3054a158d9c6fad2e5641b7d5207a951287b576/test/integration/utilities_test.go. That said, recently we talked about moving this kind of code to a place which will be shared with e2e. I think I'm doing the same "start if not running" check here:
crc/test/e2e/testsuite/testsuite.go
Line 583 in e3054a1
extended we can later refactor it to be used in multiple places. Again, up to you.
| Expect(exec.Command("pmset", "sleepnow").Run()).To(Succeed()) | ||
| By("getting dates from instance and host") | ||
| out, err := exec.Command("date", "-u", "+%s").Output() | ||
| Expect(err).To(BeEmpty()) |
There was a problem hiding this comment.
I think elsewhere I'm using Expect(err).NotTo(HaveOccurred()). Can stay as it is if you prefer it.
|
|
||
| var _ = Describe("Guest machine time should be in sync with host", Label("guest-timesync", "darwin"), func() { | ||
|
|
||
| BeforeSuite(func() { |
There was a problem hiding this comment.
The way I understand BeforeSuite is that it acts globally (before each Describe block?), so it should probably be defined outside of it too, e.g. in testsuite_test.go. Here, maybe BeforeEach would work better?
That said, I am not sure if the It block wouldn't be better split into a few of them, in which case BeforeEach wouldn't work so well.
| } | ||
| }) | ||
|
|
||
| It("checks instance date is in sync with host after host suspension", func() { |
There was a problem hiding this comment.
I'm not sure what good practice is in Ginkgo, but I imagine It blocks to be smaller. One of the reasons for this is that the results file is iterated by It blocks. If only one block present, then only one pass/fail result. Which is probably good for e2e test (?) but not sure about integration... More detail in the results might be easier to debug too. But I'm not an expert here, just suggesting!
|
I am keeping this PR on hold as I have been trying to ensure we can mimic the sleep mode on our qe envs, but I was not able for the moment. On |
|
Close as I am not able to reproduce the scenario on remote machines |
This test ensures the timesync feature works as expected for darwin platform