Skip to content

Add Warnings for Missing Interfaces in Device Introspection#486

Open
AmerMesanovic wants to merge 2 commits intoastarte-platform:masterfrom
AmerMesanovic:interface-not-installed
Open

Add Warnings for Missing Interfaces in Device Introspection#486
AmerMesanovic wants to merge 2 commits intoastarte-platform:masterfrom
AmerMesanovic:interface-not-installed

Conversation

@AmerMesanovic
Copy link
Copy Markdown
Contributor

Added a warning indicator for interfaces not installed in the realm. Links are removed for such interfaces.

Screens:
Screenshot from 2024-12-31 13-24-33
Screenshot from 2024-12-31 13-24-24

Copy link
Copy Markdown
Collaborator

@Pavinati Pavinati left a comment

Choose a reason for hiding this comment

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

You'll also have to change cypress tests regarding this page to catch the additional call to the installed interfaces

@AmerMesanovic AmerMesanovic force-pushed the interface-not-installed branch from 4521dd4 to 5cef66e Compare January 20, 2025 13:03
@AmerMesanovic AmerMesanovic force-pushed the interface-not-installed branch 7 times, most recently from 60f7903 to cb7ab5a Compare January 22, 2025 15:23
Comment on lines +81 to +90
const isInterfaceInstalled = interfacesData.some(
(interfaceData) => interfaceData.name === iface.name,
);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should verify whether the exact interface is installed, so we should check for .major and .minor besides just .name.

});
});

it.only('displays a warning when the device introspection contains an uninstalled interface', function () {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's not use it.only, otherwise the test runner will only run this test.

Comment on lines +107 to +121
Cypress.Commands.add('checkInterfaceWarning', (interfaceName, interfacesData) => {
const interfaceNameString = typeof interfaceName === 'object' ? JSON.stringify(interfaceName) : interfaceName;
const isInterfaceInInterfaces = interfacesData.some(i => i.name.trim().toLowerCase() === interfaceNameString.trim().toLowerCase());
cy.contains(interfaceNameString)
.parent()
.find('span.d-inline-flex')
.then(($badge) => {
if (isInterfaceInInterfaces) {
cy.wrap($badge).should('not.contain', '!');
} else {
cy.wrap($badge).should('exist');
cy.wrap($badge).should('contain', '!');
}
});
});
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Defining a new Cypress command can be useful for generic and reusable logic, however here you have a piece of code that is really targeting a specific test. I think it would be better to just write these few lines within the test directly, so it's easier to follow the logic within the same context.

Specifically, since in the test we want to verify a specific condition (that a warning is displayed when the interface is not installed), I suggest being explicit and test just that.
Right now it's not straightforward which interface should generate a warning and which not (if any).

We could setup the fixtures with just not existing interfaces, and then verify only the condition that a warning is shown.

Added a warning indicator for interfaces not installed in the realm.
Links are removed for such interfaces.

Signed-off-by: AmerMesanovic <amer.mesanovic@secomind.com>
@AmerMesanovic AmerMesanovic force-pushed the interface-not-installed branch 2 times, most recently from a7dd8a9 to ec7bd50 Compare February 11, 2025 14:11
@AmerMesanovic AmerMesanovic force-pushed the interface-not-installed branch 2 times, most recently from fd11d8a to 73d85f9 Compare February 11, 2025 14:34
Introduced a test to verify that a warning is displayed when a device's
introspection includes an interface that is not installed.

Signed-off-by: AmerMesanovic <amer.mesanovic@secomind.com>
@AmerMesanovic AmerMesanovic force-pushed the interface-not-installed branch from 73d85f9 to 0c4ceaa Compare February 11, 2025 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants