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

[integration] timesync test #3461

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions test/integration/timesync_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package test_test

import (
"os/exec"
"strconv"

// "github.com/crc-org/crc/test/extended/crc/cmd".
Copy link

Choose a reason for hiding this comment

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

I guess this line can be removed?

"github.com/crc-org/crc/test/extended/crc/cmd"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

const (
sleepSeconds = "30"
maxDateDiffInSeconds = 10
)

var _ = Describe("Guest machine time should be in sync with host", Label("guest-timesync", "darwin"), func() {

BeforeSuite(func() {
Copy link

Choose a reason for hiding this comment

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

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("Ensuring instance is running")
if err := cmd.CheckCRCStatus("running"); err != nil {
// cluster is not running
startInstance()
}
})

It("checks instance date is in sync with host after host suspension", func() {
Copy link

Choose a reason for hiding this comment

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

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!

By("scheduling a suspensions and resume of the host")
Expect(exec.Command("pmset", "relative", sleepSeconds).Run()).To(Succeed())
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())
Copy link

Choose a reason for hiding this comment

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

I think elsewhere I'm using Expect(err).NotTo(HaveOccurred()). Can stay as it is if you prefer it.

hostDate := string(out)
instanceDate, err := SendCommandToVM("date -u +%s")
Expect(err).To(BeEmpty())
By("ensuring date matches on host and instance")
hostDateAsInt, err := strconv.Atoi(hostDate)
Expect(err).To(BeEmpty())
instanceDateAsInt, err := strconv.Atoi(instanceDate)
Expect(err).To(BeEmpty())
dateDiff := instanceDateAsInt - hostDateAsInt
Ω(dateDiff).Should(BeNumerically("<=", maxDateDiffInSeconds))
})

AfterSuite(func() {
// delete instance cleanup...
It("Cleanup instance")
Expect(cmd.CRC("cleanup").Execute()).To(Succeed())
})

})
18 changes: 18 additions & 0 deletions test/integration/util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package test_test

import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func startInstance() {
Copy link

@jsliacan jsliacan Jan 9, 2023

Choose a reason for hiding this comment

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

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.

Copy link

Choose a reason for hiding this comment

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

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:

func EnsureCRCIsRunningSucceedsOrFails(expected string) error {
Maybe if you put this in extended we can later refactor it to be used in multiple places. Again, up to you.

// cluster is not running
if bundlePath == "" {
Expect(RunCRCExpectSuccess("setup")).To(ContainSubstring("Your system is correctly setup for using CRC"))
} else {
Expect(RunCRCExpectSuccess("setup", "-b", bundlePath)).To(ContainSubstring("Your system is correctly setup for using CRC"))
}
It("start CRC", func() {
Expect(RunCRCExpectSuccess("start", "-p", pullSecretPath)).To(ContainSubstring("Started the OpenShift cluster"))
})
}