Skip to content

Commit 2ecad6f

Browse files
AryaHassanliAryaHassanli
authored
Merge pull request #59 from AryaHassanli/fix-provisionality-parent-resolving
Skip elements with provisional ancestors
2 parents e57be18 + b6b0c0c commit 2ecad6f

14 files changed

Lines changed: 363 additions & 0 deletions

File tree

‎provisional/compare.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ func compare(specs spec.SpecPullRequest) (violationsByPath map[string][]spec.Vio
6060
parent = parent.Parent()
6161
}
6262

63+
// When an ancestor is provisional, we don't need to report non-provisional violations for this entity
64+
if parent != nil {
65+
violationType &= ^spec.ViolationTypeNonProvisional
66+
if violationType == spec.ViolationTypeNone {
67+
continue
68+
}
69+
}
70+
6371
v := spec.Violation{Entity: entity, Type: violationType}
6472
v.Path, v.Line = entity.Origin()
6573
violationsByPath[v.Path] = append(violationsByPath[v.Path], v)

‎provisional/compare_test.go‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package provisional_test
2+
3+
import (
4+
"context"
5+
"log/slog"
6+
"os"
7+
"testing"
8+
9+
"github.com/project-chip/alchemy/internal/pipeline"
10+
"github.com/project-chip/alchemy/matter/spec"
11+
"github.com/project-chip/alchemy/provisional"
12+
)
13+
14+
func TestCompareProvisional_FileBased_Cases(t *testing.T) {
15+
slog.SetDefault(slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})))
16+
17+
cxt := context.Background()
18+
baseRoot := "testdata/base"
19+
headRoot := "testdata/head"
20+
21+
processingOptions := pipeline.ProcessingOptions{}
22+
23+
violations, err := provisional.Pipeline(cxt, baseRoot, headRoot, nil, processingOptions, nil)
24+
if err != nil {
25+
t.Fatalf("Pipeline failed: %v", err)
26+
}
27+
28+
// We read test_cluster.adoc which contains 4 cases:
29+
//
30+
// Case 1: ProvCommand1 and Field1
31+
// - ProvCommand1 is an existing provisional command (matched by ID 0x00 to base).
32+
// - Field1 is a NEW non-provisional field added to it, inside if-def.
33+
// - Field1 triggers NonProvisional violation, but it is MASKED by provisional parent ProvCommand1.
34+
// - Result: PASSES. Verifies ancestor resolving logic.
35+
//
36+
// Case 2: ProvCommand2
37+
// - Marked provisional (P) in table.
38+
// - NOT inside ifdef::in-progress[].
39+
// - Result: FAILS (NotIfDefd). Provisional entities must be in ifdef.
40+
//
41+
// Case 3: ProvCommand3
42+
// - Marked provisional (P) in table.
43+
// - Inside ifdef::in-progress[].
44+
// - Result: PASSES. It is properly ifdef'd.
45+
//
46+
// Case 4: NonProvCommand
47+
// - Marked mandatory (M) (not provisional) in table.
48+
// - Inside ifdef::in-progress[].
49+
// - Result: FAILS (NonProvisional). Non-provisional entities should not be in in-progress ifdef.
50+
51+
expectedNotIfDefd := 0
52+
expectedNonProvisional := 0
53+
54+
for _, vs := range violations {
55+
for _, v := range vs {
56+
if v.Type.Has(spec.ViolationTypeNotIfDefd) {
57+
expectedNotIfDefd++
58+
}
59+
if v.Type.Has(spec.ViolationTypeNonProvisional) {
60+
expectedNonProvisional++
61+
}
62+
}
63+
}
64+
65+
// We expect 1 NotIfDefd: for ProvCommand2 (no ifdef).
66+
if expectedNotIfDefd != 1 {
67+
t.Errorf("Expected 1 NotIfDefd violations, got %d", expectedNotIfDefd)
68+
}
69+
70+
// We expect 1 NonProvisional: for NonProvCommand (Case 4: not provisional in ifdef).
71+
if expectedNonProvisional != 1 {
72+
t.Errorf("Expected 1 NonProvisional violation, got %d", expectedNonProvisional)
73+
}
74+
}

‎provisional/testdata/base/src/app_clusters/test_cluster.adoc‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
[[ref_TestCluster, Test Cluster]]
2+
= Test Cluster
3+
4+
== Classification
5+
6+
|===
7+
| Hierarchy | Role | Scope | PICS Code
8+
| Base | Application | Endpoint | TEST
9+
|===
10+
11+
== Cluster Identifiers
12+
13+
[options="header",valign="middle"]
14+
|===
15+
| Identification | Name
16+
| 0xFFFE | Test Cluster
17+
|===
18+
19+
== Commands
20+
[options="header",valign="middle"]
21+
|===
22+
| ID | Name | Direction | Response | Access | Conformance
23+
| 0x00 s| TestCommand | client => server | Y | O | P
24+
|===

‎provisional/testdata/base/src/device_types/BaseDeviceType.adoc‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
[[ref_BaseDeviceType, Base Device Type]]
2+
= Base Device Type
3+
4+
== Classification
5+
[options="header",valign="middle"]
6+
|===
7+
| Device Name | Device Type ID | Device Type Revision
8+
| Base Device Type | 0xFFFE | 1
9+
|===
10+
11+
== Cluster Requirements
12+
[options="header",valign="middle"]
13+
|===
14+
| Cluster | Client/Server | Conformance
15+
| Basic Information | Server | M
16+
|===

‎provisional/testdata/base/src/device_types/RootNode.adoc‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
[[ref_RootNode, Root Node]]
2+
= Root Node
3+
4+
== Classification
5+
[options="header",valign="middle"]
6+
|===
7+
| Device Name | Device Type ID | Device Type Revision
8+
| Root Node | 0x0016 | 1
9+
|===
10+
11+
== Cluster Requirements
12+
[options="header",valign="middle"]
13+
|===
14+
| Cluster | Client/Server | Conformance
15+
| Basic Information | Server | M
16+
|===

‎provisional/testdata/base/src/main.adoc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
= Main Spec
2+
3+
include::service_device_management/BasicInformationCluster.adoc[]
4+
include::service_device_management/BridgedDeviceBasicInformationCluster.adoc[]
5+
include::device_types/RootNode.adoc[]
6+
include::device_types/BaseDeviceType.adoc[]
7+
include::app_clusters/test_cluster.adoc[]

‎provisional/testdata/base/src/service_device_management/BasicInformationCluster.adoc‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
[[ref_BasicInformationCluster, Basic Information Cluster]]
2+
= Basic Information Cluster
3+
4+
== Classification
5+
6+
|===
7+
| Hierarchy | Role | Scope | PICS Code
8+
| Base | Application | Endpoint | BIS
9+
|===
10+
11+
== Cluster Identifiers
12+
13+
[options="header",valign="middle"]
14+
|===
15+
| Identification | Name
16+
| 0x0028 | Basic Information
17+
|===
18+
19+
== Revision History
20+
21+
[options="header",valign="middle"]
22+
|===
23+
| Revision | Description
24+
| 1 | Initial Release
25+
|===

‎provisional/testdata/base/src/service_device_management/BridgedDeviceBasicInformationCluster.adoc‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
[[ref_BridgedDeviceBasicInformationCluster, Bridged Device Basic Information Cluster]]
2+
= Bridged Device Basic Information Cluster
3+
4+
== Classification
5+
6+
|===
7+
| Hierarchy | Role | Scope | PICS Code
8+
| Base | Application | Endpoint | BDBIS
9+
|===
10+
11+
== Cluster Identifiers
12+
13+
[options="header",valign="middle"]
14+
|===
15+
| Identification | Name
16+
| 0x0039 | Bridged Device Basic Information
17+
|===
18+
19+
== Revision History
20+
21+
[options="header",valign="middle"]
22+
|===
23+
| Revision | Description
24+
| 1 | Initial Release
25+
|===

‎provisional/testdata/head/src/app_clusters/test_cluster.adoc‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
[[ref_TestCluster, Test Cluster]]
2+
= Test Cluster
3+
4+
== Classification
5+
6+
|===
7+
| Hierarchy | Role | Scope | PICS Code
8+
| Base | Application | Endpoint | TEST
9+
|===
10+
11+
== Cluster Identifiers
12+
13+
[options="header",valign="middle"]
14+
|===
15+
| Identification | Name
16+
| 0xFFFE | Test Cluster
17+
|===
18+
19+
== Commands
20+
[options="header",valign="middle"]
21+
|===
22+
| ID | Name | Direction | Response | Access | Conformance
23+
ifdef::in-progress[]
24+
| 0x00 s| ProvCommand1 | client => server | Y | O | P
25+
endif::[]
26+
| 0x01 s| ProvCommand2 | client => server | Y | O | P
27+
ifdef::in-progress[]
28+
| 0x02 s| ProvCommand3 | client => server | Y | O | P
29+
endif::[]
30+
ifdef::in-progress[]
31+
| 0x03 s| NonProvCommand | client => server | Y | O | M
32+
endif::[]
33+
|===
34+
35+
ifdef::in-progress[]
36+
=== ProvCommand1 Command
37+
This is Case 1: Provisional in ifdef with no provisional ancestor.
38+
(The cluster is NOT provisional).
39+
40+
[options="header",valign="middle"]
41+
|===
42+
| ID | Name | Type | Constraint | Quality | Default | Conformance
43+
| 0 s| Field1 | string | max 6 | | empty | M
44+
|===
45+
endif::[]
46+
47+
=== ProvCommand2 Command
48+
This is Case 2: Provisional not in ifdef.
49+
(It is in the table without ifdef).
50+
51+
[options="header",valign="middle"]
52+
|===
53+
| ID | Name | Type | Constraint | Quality | Default | Conformance
54+
| 0 s| Field2 | string | max 6 | | empty | M
55+
|===
56+
57+
ifdef::in-progress[]
58+
=== ProvCommand3 Command
59+
This is Case 3: Provisional in ifdef with provisional ancestor.
60+
(ProvCommand3 is provisional, and Field3 inside it is also provisional).
61+
62+
[options="header",valign="middle"]
63+
|===
64+
| ID | Name | Type | Constraint | Quality | Default | Conformance
65+
| 0 s| Field3 | string | max 6 | | empty | P
66+
|===
67+
endif::[]
68+
69+
ifdef::in-progress[]
70+
=== NonProvCommand Command
71+
This is Case 4: Not provisional in ifdef.
72+
(Triggers NonProvisional violation).
73+
74+
[options="header",valign="middle"]
75+
|===
76+
| ID | Name | Type | Constraint | Quality | Default | Conformance
77+
| 0 s| Field4 | string | max 6 | | empty | M
78+
|===
79+
endif::[]

‎provisional/testdata/head/src/device_types/BaseDeviceType.adoc‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
[[ref_BaseDeviceType, Base Device Type]]
2+
= Base Device Type
3+
4+
== Classification
5+
[options="header",valign="middle"]
6+
|===
7+
| Device Name | Device Type ID | Device Type Revision
8+
| Base Device Type | 0xFFFE | 1
9+
|===
10+
11+
== Cluster Requirements
12+
[options="header",valign="middle"]
13+
|===
14+
| Cluster | Client/Server | Conformance
15+
| Basic Information | Server | M
16+
|===

0 commit comments

Comments
 (0)