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

Add Unit Tests for Element Conformance Warnings in ZAP File Import #1518

Merged
merged 4 commits into from
Feb 18, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions test/importexport.test.js
ethanzhouyc marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ let provisionalCluster = path.join(
__dirname,
'resource/test-provisional-cluster.zap'
)
let nonConformElements = path.join(
__dirname,
'resource/non-conform-elements.zap'
)

// Due to future plans to rework how we handle global attributes,
// we introduce this flag to bypass those attributes when testing import/export.
Expand Down Expand Up @@ -470,3 +474,45 @@ test(
},
testUtil.timeout.medium()
)

test(
'Import a ZAP file with elements that do not conform to a device type feature and make sure element conformance warnings show up in the notification table',
async () => {
sid = await querySession.createBlankSession(db)
await util.ensurePackagesAndPopulateSessionOptions(
templateContext.db,
sid,
{
zcl: env.builtinSilabsZclMetafile(),
template: env.builtinTemplateMetafile()
},
null,
[templatePkgId]
)
await importJs.importDataFromFile(db, nonConformElements, {
sessionId: sid
})
expect(sid).not.toBeUndefined()
let notifications = await sessionNotification.getNotification(db, sid)
let notificationMessages = notifications.map((not) => not.message)

// Two attributes and one command conform to the enabled device type feature 'LT' but are disabled.
// Their element conformance warnings should be added to the notification table.
expect(
notificationMessages.includes(
'On endpoint 1, cluster: On/Off, command: OffWithEffect conforms to LT and is mandatory based on state of feature: LT.'
Copy link
Collaborator

Choose a reason for hiding this comment

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

This message seems very confusing on what the user needs to(should) do. Change it to
On endpoint 1, cluster: On/Off, [incoming or outgoing] command: OffWithEffect needs to be enabled to ensure mandatory conformance with the enabled Lighting(LT) feature.

Or

'On endpoint 1, cluster: On/Off, [incoming or outgoing] command: OffWithEffect has mandatory conformance to Lighting(LT) feature and should be enabled when LT is enabled'

Copy link
Collaborator Author

@ethanzhouyc ethanzhouyc Feb 14, 2025

Choose a reason for hiding this comment

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

Complicated conformance expressions have to be considered in the message, for example, LT | AB | element1. I will improve the message as below to cover all cases:

'On endpoint 1, cluster: On/Off, [incoming or outgoing] command: OffWithEffect has mandatory conformance to LT | AB | element1 and should be enabled when feature LT is enabled, feature AB is disabled, and element element1 is disabled'.

)
).toBeTruthy()
expect(
notificationMessages.includes(
'On endpoint 1, cluster: On/Off, attribute: OnTime conforms to LT and is mandatory based on state of feature: LT.'
ethanzhouyc marked this conversation as resolved.
Show resolved Hide resolved
)
).toBeTruthy()
expect(
notificationMessages.includes(
'On endpoint 1, cluster: On/Off, attribute: GlobalSceneControl conforms to LT and is mandatory based on state of feature: LT.'
)
).toBeTruthy()
},
testUtil.timeout.medium()
)
35 changes: 31 additions & 4 deletions test/multi-protocol.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,37 @@ test(
)
).toBeTruthy()

// Just one notification regarding multiple top level zcl propertoes and 4
// notifications regarding feature map attribute not set correctly
// and one notification regarding the enabled provisional cluster: Scenes
expect(sessionNotifications.length).toEqual(6)
// Tests for provisional cluster warnings
expect(
sessionNotificationMessages.includes(
'On endpoint 1, support for cluster: Scenes server is provisional.'
)
).toBeTruthy()

// Tests for the attributes and commands that do not conform to the device type feature LT
let nonConformElements = [
{ name: 'StartUpOnOff', type: 'attribute' },
{ name: 'OffWaitTime', type: 'attribute' },
{ name: 'OnTime', type: 'attribute' },
{ name: 'GlobalSceneControl', type: 'attribute' },
{ name: 'OffWithEffect', type: 'command' },
{ name: 'OnWithRecallGlobalScene', type: 'command' },
{ name: 'OnWithTimedOff', type: 'command' }
]

for (const element of nonConformElements) {
expect(
sessionNotificationMessages.includes(
`On endpoint 1, cluster: On/Off, ${element.type}: ${element.name} conforms to LT and is not supported based on state of feature: LT.`
)
).toBeTruthy()
}

// one notification regarding multiple top level zcl propertoes
// 4 notifications regarding feature map attribute not set correctly
// one notification regarding the enabled provisional cluster
// 7 notifications regarding non-conformed elements
expect(sessionNotifications.length).toEqual(13)

// Test Accumulators in templates
let zigbeeEndpointEvents = genResultZigbee.content['zap-event.h']
Expand Down
Loading