-
Notifications
You must be signed in to change notification settings - Fork 32
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
[Cypress] Fix failed test on v1.4.0 for the dashboard string changes #1588
[Cypress] Fix failed test on v1.4.0 for the dashboard string changes #1588
Conversation
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.
Just the one change and other than that LGTM. Thanks for updating all of the strings. I was seeing the same problems in my runs.
type: 'S3', | ||
endpoint: "http://172.19.99.205:9000", | ||
bucketName: "cypress", | ||
bucketRegion: "ecmlab", | ||
accessKeyId: "minioadmin", | ||
secretAccessKey: "minioadmin", |
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.
This looks like you hardcoded in some stuff instead of grabbing from the config/env file. You should change that.
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.
Thanks for the check.
I have change back the S3 backup target setting.
Since we are migrating test environment to the new lab machines. I thinks it would be better to create a new PR to handle with S3 backup testcases along with the new minio settings.
@TachunLin you should also squash your commits before the PR merge. |
a22ae3d
to
5d505ca
Compare
be8e2e2
to
fb04229
Compare
I just S3 backup target in the settings/setting.spec.ts and squash the commit. |
fb04229
to
fb7a070
Compare
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.
LGTM
Which issue(s) this PR fixes:
Harvester issue: harvester/harvester#6407
Test sssue: #1535
What this PR does / why we need it:
Starting with v1.4.0, we have made some dashboard string changes in issue [BUG] UI Improvements: Language and consistency harvester#6407
This change would cause cypress test to have around
46
failure result job #110The corresponding test result:
Special notes for your reviewer:
Tested on the latest
v1.4.0-rc1
cypress test result on Jenkins Harvester cypress test job # 114According to the corresponding cypress test report
After the fix, the failed test reduced from
46
to9