Skip to content

Commit

Permalink
lib: Fix ListingTable nesting for non-expandable rows
Browse files Browse the repository at this point in the history
`<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.
  • Loading branch information
martinpitt committed Oct 24, 2023
1 parent a43a0be commit d178acf
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 45 deletions.
7 changes: 2 additions & 5 deletions pkg/lib/cockpit-components-table.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,7 @@ export const ListingTable = ({
</React.Fragment>
);

if (row.expandedContent)
return <Tbody key={rowKey} isExpanded={isExpanded}>{rowPair}</Tbody>;
else
return rowPair;
return <Tbody key={rowKey} isExpanded={row.expandedContent && isExpanded}>{rowPair}</Tbody>;
});

return (
Expand Down Expand Up @@ -293,7 +290,7 @@ export const ListingTable = ({
})}
</Tr>
</Thead>}
{!isExpandable ? <Tbody>{rowsComponents}</Tbody> : rowsComponents}
{rowsComponents}
</Table>
</>
);
Expand Down
8 changes: 4 additions & 4 deletions test/verify/check-shell-keys
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,11 @@ class TestKeys(testlib.MachineCase):

# Add invalid key directly
m.write("/home/user/.ssh/authorized_keys", "\nbad\n", append=True)
b.wait_in_text("#account-authorized-keys-list tr:last-child", "Invalid key")
b.wait_in_text("#account-authorized-keys-list tbody:last-child", "Invalid key")
b.wait_js_func("ph_count_check", "#account-authorized-keys-list tr", 2)

# Removing the key
b.click("#account-authorized-keys-list tr:last-child button")
b.click("#account-authorized-keys-list tbody:last-child button")
b.wait_not_in_text("#account-authorized-keys", "Invalid key")
b.wait_js_func("ph_count_check", "#account-authorized-keys-list tr", 1)
data = m.execute("cat /home/user/.ssh/authorized_keys")
Expand All @@ -128,9 +128,9 @@ class TestKeys(testlib.MachineCase):

# User can still see their keys
login("user")
b.wait_in_text("#account-authorized-keys-list tr:first-child", "test-name")
b.wait_in_text("#account-authorized-keys-list tbody:first-child", "test-name")

b.click("#account-authorized-keys-list tr:first-child button")
b.click("#account-authorized-keys-list tbody:first-child button")
b.wait_in_text("#account-authorized-keys", "no authorized public keys")

self.allow_journal_messages('authorized_keys is not a public key file.')
Expand Down
48 changes: 24 additions & 24 deletions test/verify/check-system-info
Original file line number Diff line number Diff line change
Expand Up @@ -519,17 +519,17 @@ class TestSystemInfo(packagelib.PackageCase):
# PCI
b.wait_in_text(pci_selector + heading_selector, "PCI")

b.wait_in_text(pci_selector + ' tr:first-of-type td[data-label=Slot]', "0000:00:00.0")
b.wait_in_text(pci_selector + ' tbody:first-of-type td[data-label=Slot]', "0000:00:00.0")

# sorted by device class by default; this makes some assumptions about QEMU devices
b.wait_in_text(pci_selector + ' tbody tr:first-of-type td[data-label=Class]', "Bridge")
b.wait_in_text(pci_selector + ' tbody tr:last-of-type td[data-label=Class]', "Unclassified")
b.wait_in_text(pci_selector + ' tbody:first-of-type td[data-label=Class]', "Bridge")
b.wait_in_text(pci_selector + ' tbody:last-of-type td[data-label=Class]', "Unclassified")

# sort by model
b.click(pci_selector + ' thead th:nth-child(2) button')
b.wait_in_text(pci_selector + ' tbody tr:first-of-type td[data-label=Model]', "440")
b.wait_in_text(pci_selector + ' tbody tr:last-of-type td[data-label=Model]', "Virtio SCSI")
b.wait_not_in_text(pci_selector + ' tbody tr:last-of-type td[data-label=Model]', "Unclassified")
b.wait_in_text(pci_selector + ' tbody:first-of-type td[data-label=Model]', "440")
b.wait_in_text(pci_selector + ' tbody:last-of-type td[data-label=Model]', "Virtio SCSI")
b.wait_not_in_text(pci_selector + ' tbody:last-of-type td[data-label=Model]', "Unclassified")

# go back to system page
b.click('.pf-v5-c-breadcrumb li:first-of-type')
Expand Down Expand Up @@ -557,7 +557,7 @@ class TestSystemInfo(packagelib.PackageCase):

# PCI should be shown
b.wait_in_text(pci_selector + heading_selector, "PCI")
b.wait_in_text(pci_selector + ' tr:first-of-type td[data-label=Slot]', "0000:00:00.0")
b.wait_in_text(pci_selector + ' tbody:first-of-type td[data-label=Slot]', "0000:00:00.0")

# Check also variants when only some fields are present
m.write("/sys/class/dmi/id/chassis_type", "10")
Expand Down Expand Up @@ -688,27 +688,27 @@ machine : 8561
distros_without_systemd_memory_dmi = ['rhel-8-7', 'rhel-8-8', 'rhel-8-9', 'centos-8-stream']

# Test more specific memory data with a fake dmidecode
b.wait_in_text('#memory-listing tr:nth-of-type(1) td[data-label=ID]', "BANK 0: ChannelA-DIMM0")
b.wait_in_text('#memory-listing tr:nth-of-type(1) td[data-label=Type]', "DDR4")
b.wait_in_text('#memory-listing tbody:nth-of-type(1) td[data-label=ID]', "BANK 0: ChannelA-DIMM0")
b.wait_in_text('#memory-listing tbody:nth-of-type(1) td[data-label=Type]', "DDR4")
if m.image in distros_without_systemd_memory_dmi:
b.wait_in_text('#memory-listing tr:nth-of-type(1) td[data-label=Size]', "4 GB")
b.wait_in_text('#memory-listing tbody:nth-of-type(1) td[data-label=Size]', "4 GB")
else:
b.wait_in_text('#memory-listing tr:nth-of-type(1) td[data-label=Size]', "4 GiB")
b.wait_in_text('#memory-listing tr:nth-of-type(1) td[data-label=State]', "Present")
b.wait_text('#memory-listing tr:nth-of-type(1) td[data-label="Memory technology"]', "Unknown")
b.wait_text('#memory-listing tr:nth-of-type(1) td[data-label=Rank]', "Single rank")
b.wait_in_text('#memory-listing tr:nth-of-type(1) td[data-label=Speed]', "2400 MT/s")

b.wait_in_text('#memory-listing tr:nth-of-type(2) td[data-label=ID]', "BANK 2: ChannelB-DIMM0")
b.wait_in_text('#memory-listing tr:nth-of-type(2) td[data-label=Type]', "DDR4")
b.wait_in_text('#memory-listing tbody:nth-of-type(1) td[data-label=Size]', "4 GiB")
b.wait_in_text('#memory-listing tbody:nth-of-type(1) td[data-label=State]', "Present")
b.wait_text('#memory-listing tbody:nth-of-type(1) td[data-label="Memory technology"]', "Unknown")
b.wait_text('#memory-listing tbody:nth-of-type(1) td[data-label=Rank]', "Single rank")
b.wait_in_text('#memory-listing tbody:nth-of-type(1) td[data-label=Speed]', "2400 MT/s")

b.wait_in_text('#memory-listing tbody:nth-of-type(2) td[data-label=ID]', "BANK 2: ChannelB-DIMM0")
b.wait_in_text('#memory-listing tbody:nth-of-type(2) td[data-label=Type]', "DDR4")
if m.image in distros_without_systemd_memory_dmi:
b.wait_in_text('#memory-listing tr:nth-of-type(1) td[data-label=Size]', "4 GB")
b.wait_in_text('#memory-listing tbody:nth-of-type(1) td[data-label=Size]', "4 GB")
else:
b.wait_in_text('#memory-listing tr:nth-of-type(1) td[data-label=Size]', "4 GiB")
b.wait_in_text('#memory-listing tr:nth-of-type(2) td[data-label=State]', "Present")
b.wait_text('#memory-listing tr:nth-of-type(2) td[data-label="Memory technology"]', "Unknown")
b.wait_text('#memory-listing tr:nth-of-type(2) td[data-label=Rank]', "Single rank")
b.wait_in_text('#memory-listing tr:nth-of-type(2) td[data-label=Speed]', "2400 MT/s")
b.wait_in_text('#memory-listing tbody:nth-of-type(1) td[data-label=Size]', "4 GiB")
b.wait_in_text('#memory-listing tbody:nth-of-type(2) td[data-label=State]', "Present")
b.wait_text('#memory-listing tbody:nth-of-type(2) td[data-label="Memory technology"]', "Unknown")
b.wait_text('#memory-listing tbody:nth-of-type(2) td[data-label=Rank]', "Single rank")
b.wait_in_text('#memory-listing tbody:nth-of-type(2) td[data-label=Speed]', "2400 MT/s")

@ testlib.nondestructive
def testCPUSecurityMitigationsDetect(self):
Expand Down
13 changes: 6 additions & 7 deletions test/verify/check-system-services
Original file line number Diff line number Diff line change
Expand Up @@ -345,17 +345,16 @@ WantedBy=default.target
# returns index of first unit that isn't failed
b.inject_js("""
function firstWorkingUnitPos() {
const arr = document.getElementById('services-list').firstChild;
for (var i = 0; i < arr.childElementCount; i++) {
const child = arr.children[i];
if (!child.classList.contains('service-unit-failed')) {
return i + 1;
}
const services = document.getElementById('services-list');
for (let i = 0; i < services.childElementCount; i++) {
const tbody = services.children[i];
if (!tbody.firstChild.classList.contains('service-unit-failed'))
return i;
}
}
""")
pos = b.eval_js('firstWorkingUnitPos();')
b.wait_text(f'#services-list > tbody > tr:nth-child({pos}) > th > div > div > a', 'test')
b.wait_text(f'#services-list > tbody:nth-child({pos + 1}) > tr > th > div > div > a', 'test')
b.is_present('#test.service > svg.service-thumbtack-icon-color')

# Unpin unit
Expand Down
10 changes: 5 additions & 5 deletions test/verify/check-users
Original file line number Diff line number Diff line change
Expand Up @@ -325,15 +325,15 @@ class TestAccounts(testlib.MachineCase):

# test filters
b.set_input_text("#accounts-filter input", "root")
b.wait_in_text("#accounts-list tbody tr:first-child td[data-label='Username']", "root")
b.wait_in_text("#accounts-list tbody:first-of-type td[data-label='Username']", "root")
b.set_input_text("#accounts-filter input", "rOBeRt")
b.wait_in_text("#accounts-list tbody tr:first-child td[data-label='Username']", "robert")
b.wait_in_text("#accounts-list tbody:first-of-type td[data-label='Username']", "robert")

uid = "1000"
if "debian" in m.image or "ubuntu" in m.image:
uid = "1001"
b.set_input_text("#accounts-filter input", uid)
b.wait_in_text("#accounts-list tbody tr:first-child td[data-label='ID']", uid)
b.wait_in_text("#accounts-list tbody:first-of-type td[data-label='ID']", uid)
b.set_input_text("#accounts-filter input", "spooky")
b.wait_visible("#accounts div.pf-v5-c-empty-state")

Expand All @@ -346,7 +346,7 @@ class TestAccounts(testlib.MachineCase):

def check_column_sort(query_selector, invert=False):
# current account is always in the first row
b.wait_in_text("#accounts-list tbody tr:first-child td[data-label='Username']", "admin")
b.wait_in_text("#accounts-list tbody:first-of-type td[data-label='Username']", "admin")
values = b.eval_js(f"getTextColumn(\"{query_selector}\")")
for i in range(2, len(values)):
if values[i].isnumeric():
Expand Down Expand Up @@ -521,7 +521,7 @@ class TestAccounts(testlib.MachineCase):
b.wait_js_cond('document.querySelector("#current-account-badge").previousSibling.getAttribute("href") === "#/jussi"')

# The current account is the first in the list
b.wait_visible("#accounts-list > tbody :first-child #current-account-badge")
b.wait_visible("#accounts-list > tbody:first-of-type #current-account-badge")

# Use [x] button on the group label to remove the account from group
b.go("#/jussi")
Expand Down

0 comments on commit d178acf

Please sign in to comment.