-
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
Fix VM related test cases #1319
Conversation
9cb354d
to
6655f11
Compare
a76f30e
to
6e47deb
Compare
@@ -46,3 +46,15 @@ export function base64DecodeToBuffer(string: string) { | |||
export function base64Decode(string: string) { | |||
return !string ? string : base64DecodeToBuffer(string.replace(/[-_]/g, char => char === '-' ? '+' : '/')).toString(); | |||
} | |||
|
|||
export const nodes = { | |||
filterWitnessNode: (hosts: {name: string, witnessNode: boolean}[]) => { |
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.
filterWitnessNode()
and witnessNode
config feild are not used in the code.
Are they prepare for future test cases use?
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.
Yes, I would keep this code for future test cases.
|
||
const hostNames: string[] = hostList.map((node: Node) => node.customName || node.name); | ||
|
||
const maintenanceNodeName = hostNames[0] |
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.
Even host is prefilled with a item in cypress.env.json
.
How about add an early return error message if hostList is empty or undefined ?
Same in node-scheduling.spec.ts.
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.
Good idea, I just added a hosts util to do that.
c360ad4
to
512c1bd
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.
Code change LTGM.
Could you verify test cases pass on jenkins pipeline env again before merge PR?
Tested on the main Jenkins for this test changes along with the harvester-baremetal-ansible pull request
According to the latest test result on the staging Jenkins of:
It will encounter the Checked the |
Thanks @TachunLin . Yes, this needs a rebase. |
…t.customName config Signed-off-by: Francesco Torchia <[email protected]>
…use host.customName to check Volumes VMs; lint fix Signed-off-by: Francesco Torchia <[email protected]>
Signed-off-by: Francesco Torchia <[email protected]>
I just rebased the PR with upstream main branch. The |
Thanks for the rebase with main.
In order to test this pull request fix, I added the following content to the harvester-baremetal-ansible repo.
@torchiaf , |
@TachunLin In this scenario, the result of ansible/roles/cypress_tests/templates/cypress.env.json.j2 template, should be:
I would change the template in something like this, or declare a variable for the host-0 -> custom name and use it wherever needed:
|
Thanks @torchiaf for your suggestion, I updated the harvester-baremetal-ansible commit pull request. And trigger a new On the cypress test report: We can find the |
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.
Tested can PASS the Stop VM Negative
and VM scheduling on Specific node
on the stating Jenkins full cypress test.
https://minio.provo.rancherlabs.com:31524/cypress-test-report/cypress/results/20240618-100419-6226eb6/index.html
Thanks for fixing the issues.
Related issue #1196
The VM tests are failing because the node
harvester-node-0
has atest-custom-name
customName in Harvester configuration; This breaks the cypress tests in matching the correct UI elements.customName
.customName
in host config. This will require to update the config in Jenkins pipelines:host
should be always an arraycustomName
is empty, thename
will be used during the tests