-
Notifications
You must be signed in to change notification settings - Fork 254
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
test (e2e) : add more assertions to verify crc status output #4604
base: main
Are you sure you want to change the base?
test (e2e) : add more assertions to verify crc status output #4604
Conversation
Skipping CI for Draft Pull Request. |
933b731
to
7433400
Compare
E2E test failure is expected as it captures the failure reported in #4603 �[31mScenario: CRC start usecase�[0m �[1;30m# test/e2e/features/basic.feature:32�[0m
�[31mAnd checking the CRC status JSON output is valid�[0m �[1;30m# test/e2e/features/basic.feature:46�[0m
�[31mError: �[0m�[1;31mfailure in asserting 'ramSize' field of crc status json output, expected less than or equal to 11274289152 bytes, actual : 32680947712 bytes�[0m Now that #4603 is merged it should pass after rebasing this PR. |
7433400
to
c54baee
Compare
test/e2e/features/basic.feature
Outdated
@@ -74,4 +75,4 @@ Feature: Basic test | |||
And kubeconfig is cleaned up | |||
# cleanup | |||
When executing crc cleanup command succeeds | |||
And executing "man -P cat crc" fails | |||
# And executing "man -P cat crc" fails |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why those changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry. It looks like I forgot to uncomment it after testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I lost the changes while pushing from another machine. I'm seeing outdated version of the changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated it. I think it should be okay now.
9c4c8c9
to
40d9c08
Compare
/assign @adrianriobo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
test/e2e/testsuite/testsuite.go
Outdated
} | ||
crcStatusSuccess := crcStatusJSONOutputObj["success"] | ||
if crcStatusSuccess != true { | ||
return fmt.Errorf("failure in asserting 'success' field of crc status json output, expectd : true, actual : %t", crcStatusSuccess) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: expected *
same for other lines below
Add stricter assertions to verify output of crc status Signed-off-by: Rohan Kumar <rohaan@redhat.com>
40d9c08
to
eaa2d52
Compare
[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 |
Description
Add stricter assertions to verify output of crc status.
We only verify that the CRC status output contains the keyword
Running
orStopped
, depending on the cluster state. However, we should also verify other attributes, such asdiskSize
,ramsSize
,openshiftStatus
,cacheDir
, etc.Relates to: PR #4603
Type of change
test, version modification, documentation, etc.)
Proposed changes
Add another step in basic test scenario to verify output of
crc status -ojson
. Parse JSON output into a map and verify that values are as expected or within allowed limit.Testing
I just verified that Basic scenario E2E test is passing when #4603 is applied.
Contribution Checklist