lib: Fix ListingTable nesting for non-expandable rows #19520
Conversation
martinpitt
force-pushed
the
listing-table-nesting
branch
from
d178acf
to
0653d9d
Compare
|
The other pixel changes are correct -- these are now tbody's which look like separate "cards", as opposed to the adjoined "expandable row" ones. Same for the storage ones, so I'll accept them. |
martinpitt
force-pushed
the
listing-table-nesting
branch
from
0653d9d
to
75d160a
Compare
martinpitt
force-pushed
the
listing-table-nesting
branch
from
75d160a
to
668e64d
Compare
|
Argh, what a yak shave -- NFS kdump is flaky on RHEL 8.9 and RHEL 9.3. It's not new, it's a flake , but 3x affected retries expose it. On my todo list.. |
|
I can locally reproduce the NFS kdump failure rather well (about 1 in 2 runs). When that happens, the NFS server indeed does not have /srv/kdump/dumps/10.111.113.1* but it does have /srv/kdump/var/crash/10.111.113.1-2023-10-24-07:29:14/vmcore -- but I think that's still from the previous test which covers the default path. The config is correct: and the dialog also confirms that it got the right path. We recently had cockpit-project/bots#5263 which came and go, and bz wasn't touched by anyone except us. That seems related.. The super-annoying thing here is that this can only really be debugged with I added an extra commit which will make this much easier to investigate, and also naughty -- it will now show the whole VM console on failure in the log. Now they look like this or this. |
martinpitt
force-pushed
the
listing-table-nesting
branch
from
668e64d
to
05b107a
Compare
|
That simplification broke two tests, will fix tomorrow morning. |
The kdump boot does not appear in the journal, so it was very difficult to investigate bugs that break kdumping. Capture the console and show it on failures.
We already ascertained that `error` is defined at that point. Avoids a coverage complaint.
The Kdump dialog sets this to a `<CodeBlockCode>` aka. `<pre>` element, which cannot be a child of a `<p>`. However, we do need the `<p>` for plain strings according to the PF docs. So do a type check and support both cases.
`<Tr>` always needs to be wrapped into a `<Tbody>`. Clean up the unnecessary special cases and handle them uniformly. Adjust the tests to fix the selectors, they previously relied on the buggy behaviour. Also adjust the pixel tests -- tbodys are separated by some space.
martinpitt
force-pushed
the
listing-table-nesting
branch
from
05b107a
to
58976af
Compare
|
I restored a bit of the selector in content_row_action() that was important for disambiguation, that should fix the tests. I also added an extra commit to make the KdumpNFS flakes easier to debug. @mvollmer PTAL? |
| @@ -90,7 +90,7 @@ InlineNotification.propTypes = { | |||
| export const ModalError = ({ dialogError, dialogErrorDetail, id, isExpandable }) => { | |||
| return ( | |||
| <Alert id={id} variant='danger' isInline title={dialogError} isExpandable={!!isExpandable}> | |||
| { dialogErrorDetail && <p> {dialogErrorDetail} </p> } | |||
| { typeof dialogErrorDetail === 'string' ? <p>{dialogErrorDetail}</p> : dialogErrorDetail } | |||
There was a problem hiding this comment.
This added line is not executed by any test. Details
|
That stratis rawhide failure is entirely new and unrelated, I'll send a separate PR to fix it. |
<Tr>always needs to be wrapped into a<Tbody>.Clean up the unnecessary special cases and handle them uniformly.
Adjust the tests to fix the selectors, they previously relied on the
buggy behaviour.
Split out from PR #19509, this is already a very intrusive fix which should land separately.