Skip to content
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

First batch of React error fixes #1270

Merged
merged 9 commits into from
Oct 27, 2023
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ COCKPIT_REPO_FILES = \
$(NULL)

COCKPIT_REPO_URL = https://github.com/cockpit-project/cockpit.git
COCKPIT_REPO_COMMIT = 118d89a84afdccda8d799579b68fee2808d96a30 # 303 + 38 commits
COCKPIT_REPO_COMMIT = 0f3109953958dae3a95920217c15bf022a9a48ca # 303 + noVerticalAlign HelpIcon fix

$(COCKPIT_REPO_FILES): $(COCKPIT_REPO_STAMP)
COCKPIT_REPO_TREE = '$(strip $(COCKPIT_REPO_COMMIT))^{tree}'
Expand Down
10 changes: 3 additions & 7 deletions src/app.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function unknownConnectionName() {
}

export const App = () => {
const [loadingResources, setLoadingResources] = useState(false);
const [loadingResources, setLoadingResources] = useState(true);
const [error, setError] = useState('');
const [systemSocketInactive, setSystemSocketInactive] = useState(false);
const [virtualizationEnabled, setVirtualizationEnabled] = useState(true);
Expand All @@ -82,12 +82,8 @@ export const App = () => {

useEvent(superuser, "changed");
useEffect(() => {
initState();

setLoadingResources(true);

return unknownConnectionName()
.then(connectionNames => {
Promise.all([initState(), unknownConnectionName()])
.then(([_init, connectionNames]) => {
return Promise.allSettled(connectionNames.map(conn => {
return getLibvirtVersion({ connectionName: conn })
.then(() => {
Expand Down
5 changes: 4 additions & 1 deletion src/components/common/needsShutdown.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,10 @@ export const VmNeedsShutdown = ({ vm }) => {
position="bottom"
hasAutoWidth
bodyContent={body}>
<Label className="resource-state-text" href="javascript:null" color="blue" id={`vm-${vm.name}-needs-shutdown`} icon={<PendingIcon />}>{_("Changes pending")}</Label>
<Label className="resource-state-text" color="blue" id={`vm-${vm.name}-needs-shutdown`}
icon={<PendingIcon />} onClick={() => null}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line is not executed by any test. Details

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if dropping onClick is enough?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it. It needs to have either a href= or an onClick handler to become focusable, and I think that was the original intent for that hack.

{_("Changes pending")}
</Label>
</Popover>
);
};
4 changes: 2 additions & 2 deletions src/components/create-vm-dialog/createVmDialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ const SshKeysRow = ({
<div>{keyObject.data}</div>
</FlexItem>
: <FormGroup label={_("Public key")}
testdata={keyInvalid && "key-invalid"}
testdata={keyInvalid ? "key-invalid" : undefined}
fieldId='public-key'>
<TextArea value={item.value || ""}
aria-label={_("Public SSH key")}
Expand All @@ -817,7 +817,7 @@ const SshKeysRow = ({
<Button variant='plain'
className="btn-close"
id={id + "-btn-close"}
isSmall
size="sm"
aria-label={_("Remove item")}
icon={<TrashIcon />}
onClick={() => removeitem(idx)} />
Expand Down
2 changes: 1 addition & 1 deletion src/components/vm/disks/diskEdit.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ const CacheRow = ({ onValueChanged, dialogValues, idPrefix, shutoff }) => {
<FormSelect id={`${idPrefix}-cache-mode`}
onChange={(_event, value) => onValueChanged('cacheMode', value)}
isDisabled={!shutoff}
value={dialogValues.cacheMode}>
value={dialogValues.cacheMode ?? "default"}>
{diskCacheModes.map(cacheMode => {
return (
<FormSelectOption value={cacheMode} key={cacheMode}
Expand Down
10 changes: 5 additions & 5 deletions src/libvirtApi/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -435,11 +435,11 @@ export function getApiData({ connectionName }) {
]);
}

export function initState() {
getPythonPath();
getLoggedInUser();
getOsInfoList();
}
export const initState = () => Promise.all([
getPythonPath(),
getLoggedInUser(),
getOsInfoList(),
]);

export function usageStartPolling({
name,
Expand Down
2 changes: 1 addition & 1 deletion test/check-machines-disks
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ class TestMachinesDisks(VirtualMachinesCase):
# VIR_DOMAIN_DEVICE_MODIFY_FORCE = 4 <- is only used with 'Force eject'
# https://libvirt.org/html/libvirt-libvirt-domain.html
monitor_logs = m.execute("cat /tmp/dbus_monitor_logs")
self.test_obj.assertTrue("uint32 7" in monitor_logs)
self.test_obj.assertIn("uint32 7", monitor_logs)

return self

Expand Down
2 changes: 1 addition & 1 deletion test/check-machines-lifecycle
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class TestMachinesLifecycle(VirtualMachinesCase):

b.wait_in_text("#vm-subVmTest1-boot-order", "disk,network")
emulated_machine = b.text("#vm-subVmTest1-emulated-machine")
self.assertTrue(len(emulated_machine) > 0) # emulated machine varies across test machines
self.assertGreater(len(emulated_machine), 0) # emulated machine varies across test machines

# switch to and check Usage
b.click("#vm-subVmTest1-usage")
Expand Down
3 changes: 2 additions & 1 deletion test/check-machines-settings
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,8 @@ class TestMachinesSettings(VirtualMachinesCase):
b.wait_visible(f".boot-order-additional-info .pf-v5-c-description-list__description:contains('{pci_slot}')")
# Check a cdrom attributes are shown correctly
cdrom_row = b.text(".boot-order-list-view li:nth-child(3) .boot-order-additional-info")
self.assertTrue("cdrom" and "/var/lib/libvirt/images/phonycdrom" in cdrom_row)
self.assertIn("cdrom", cdrom_row)
self.assertIn("/var/lib/libvirt/images/phonycdrom", cdrom_row)
# Move first device down and check whether succeeded
row = b.text("#vm-subVmTest1-order-modal-device-row-1 .boot-order-additional-info")
b.click("#vm-subVmTest1-order-modal-device-row-0 #vm-subVmTest1-order-modal-down")
Expand Down
2 changes: 1 addition & 1 deletion test/check-machines-storage-pools
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ class TestMachinesStoragePools(VirtualMachinesCase):
source_path_device = m.execute(xmllint_element.format(prop="source/device/@path")).strip()
source_name = m.execute(xmllint_element.format(prop="source/name")).strip()
if "source_path" in self.source:
self.test_obj.assertTrue(self.source["source_path"] in [source_path_dir, source_path_device, source_name])
self.test_obj.assertIn(self.source["source_path"], [source_path_dir, source_path_device, source_name])
else:
self.test_obj.assertEqual("", host.rstrip())

Expand Down
10 changes: 4 additions & 6 deletions test/machineslib.py
Original file line number Diff line number Diff line change
Expand Up @@ -433,14 +433,12 @@ def setUp(self):
"Tried changing state of a disconnected RFB object",
"Failed to get libvirt version from the dbus API:.*Cannot recv data: Connection reset by peer",
# FIXME: React errors on /devel scenario; these are actual bugs!
"Warning: Failed.*type.*Invalid prop",
"Warning: Failed.*type:.*The prop .* is marked as required",
"Warning: .* prop on .* should not be null.",
"Warning: Received .* for a non-boolean attribute",
'Warning: Each child in a list should have a unique "key" prop',
"Warning: React does not recognize the .* prop on a DOM element",
"Warning: An update .* was scheduled from inside an update function",
"Warning: A future version of React will block javascript: URLs as a security precaution.",
# FIXME:: https://github.com/cockpit-project/cockpit-machines/issues/1272
"Warning: Failed.*type:.*The prop `format` is marked as required in `VolumeCreateBody`, but its value is `undefined`",
# FIXME: https://github.com/cockpit-project/cockpit-machines/issues/1273
" Warning: Failed.*type:.* Invalid prop `size` of type `string` supplied to `VolumeCreateBody`, expected `number`",
)

def downloadVmXml(self, vm):
Expand Down