-
Notifications
You must be signed in to change notification settings - Fork 15
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
Barbecue images #1803
base: main
Are you sure you want to change the base?
Barbecue images #1803
Conversation
9441943
to
6c8aa02
Compare
6c8aa02
to
6db845d
Compare
78b5a46
to
9f4a142
Compare
11e8057
to
ea153d4
Compare
d1c1144
to
d6919d6
Compare
d6919d6
to
959ff92
Compare
5629fa3
to
cf234cb
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.
Good job.
No critical blockers for this iteration.
However, besides what commented, here's a few more notes:
several checks related to corosync cmap ctl fail
error while executing corosynccmap-ctl: exec: "corosync-cmapctl": executable file not found in $PATH
package version related checks have a similar issue
error while fetching package version: package corosync is not installed exit status 1
Here /clusters/597a00db-5920-5033-b3b4-1dc98ca8718a/executions/last/373DB8/host/hana_node01
there is a failure because something can not be found in rhai
Unknown property 'value' - a getter is not registered for type '()' (line 4, position 55)
Either the check needs to cover the case or we are missing something in the fake gathered fact.
Here /clusters/597a00db-5920-5033-b3b4-1dc98ca8718a/executions/last/816815/host/hana_node01
there is a failure because
systemd gatherer not initialized properly
We need extra iterations and extra effort to get faked agent cover further scenarios and maximize their value.
'7E0221': CRITICAL, | ||
'822E47': CRITICAL, | ||
'845CC9': PASSING, | ||
A1244C: CRITICAL, |
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.
Not a blocker, but what about having all check ids as string?
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.
It's just Prettier, I will give it a try but I think the formatter will just revert every attempt of mine 👍
@@ -17,6 +17,12 @@ export const availableClusters = [ | |||
sid: '', | |||
type: 'Unknown', | |||
}, | |||
{ | |||
id: 'fa0d74a3-9240-5d9e-99fa-61c4137acf81', |
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 is hana_cluster_2
's id. It should be 597a00db-5920-5033-b3b4-1dc98ca8718a
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.
Fair enough
context('HANA scale-up checks', () => { | ||
const clusterID = 'd2522281-2c76-52dc-8500-10bdf2cc6664'; | ||
context('HANA scale-up checks', () => { | ||
const clusterID = '597a00db-5920-5033-b3b4-1dc98ca8718a'; |
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.
To avoid duplication and error we could get this id from availableClusters
['DC5429', PASSING], | ||
['F50AF5', PASSING], | ||
]; | ||
const expectedCheckResults = { |
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.
There are 7 (if I counted correctly 😄 ) missing checks from this expectation list. Since in the test we are selecting them all it might make sense to have an expectation for all of them as well.
@@ -54,7 +54,7 @@ context('Hosts Overview', () => { | |||
.then((i) => { | |||
cy.get('@hostRow') | |||
.eq(i) | |||
.should('contain', host.agentVersion.slice(0, 15)); | |||
.should('contain', host.agentVersion.slice(0, 13)); |
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.
Pls help. What did change here?
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.
Agent's version is not coming from a fixture, it's the actual version of the agent, so we must limit the slice to a fewer number of chars given that the commit sha will often change
@@ -179,13 +179,15 @@ context('Hosts Overview', () => { | |||
}); | |||
|
|||
it('should change the health to warning if saptune is not installed', () => { | |||
cy.contains('button', '2').click(); |
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.
Is this about navigating to the page where the relevant host is, since we added two?
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 👍
f9d3461
to
53cc8b2
Compare
53cc8b2
to
53bd726
Compare
53bd726
to
04df557
Compare
Description
Integrating Barbecue images in our CI for end to end tests and in our local development environment.
How was this tested?
End to end tests were updated to match new values and added where missing.