diff --git a/.github/instructions/coding-guidelines.instructions.md b/.github/instructions/coding-guidelines.instructions.md index 51d451e..6464e71 100644 --- a/.github/instructions/coding-guidelines.instructions.md +++ b/.github/instructions/coding-guidelines.instructions.md @@ -14,6 +14,26 @@ It is intended for use by both AI agents (Copilot, openspec) and human developer Markers: - `[ASSUMPTION]` — inferred from code patterns where no explicit policy exists. +- `[VERIFY]` — could not be confirmed from the repository; investigate before relying on it. Open items are collected in §18. + +--- + +## 0. Source of Truth, Precedence, and Maintenance + +When sources disagree, resolve in this order: + +1. **The official Firebolt API specification maintained in Confluence** — the single source of truth for method names, parameters, return types, error semantics, and C++17 compatibility (`C++17: ✅/❌`). Do not substitute a local export, implementation, or fixture for the specification. +2. **Merged code in this repo** — authoritative for structure and conventions. +3. **`docs/openrpc/the-spec/firebolt-open-rpc.json`** — test fixture consumed by mock-firebolt. Secondary evidence only; it can lag or diverge from the specification (see §18). +4. **This document** — a summary of the specification and repository practice; it never overrides the specification. + +Rules: +- Only implement methods marked **Approved** with `C++17: ✅` in the specification. If you cannot access it, request access or confirmation from the API owners; do not infer approval from code or fixtures. `Firebolt.apiVersion` and `ParentalControl.*` are *Not approved* and must not be implemented without owner approval. +- A method marked `Deprecated API version: 9.0.0` (all of `TextToSpeech`) is kept for compatibility. New speech work targets `SpeechSynthesis`. +- If implementation code, fixtures, or tests disagree with the specification, follow the specification for API behavior and record the discrepancy in §18. +- Revalidate this guidance when the applicable specification changes. +- **Maintenance:** when a statement here contradicts the code, fix the statement in the same change. If you cannot tell which is right, mark it `[VERIFY]` and add it to §18 rather than guessing. +- Last verified against the repository on 2026-10-14. --- @@ -30,7 +50,8 @@ The codebase has exactly three conceptual layers. Do not collapse or skip layers | JSON deserialization | `src/json_types/.h` | Wire format ↔ native type adapters; never exposed publicly | **Current practice (confirmed across all existing modules):** -- Every module has exactly one interface (`I`), one impl (`Impl`), and zero or more JSON type adapters. Four modules (discovery, localization, network, presentation) have no `json_types/` file; they use primitive-type helpers (`Firebolt::JSON::String`, `Boolean`, `Unsigned`) directly. +- There are 15 modules: accessibility, actions, advertising, device, discovery, display, lifecycle, localization, metrics, network, presentation, speechsynthesis, stats, texttospeech, videooutput. +- Every module has exactly one interface (`I`), one impl (`Impl`), and zero or more JSON type adapters. Four modules (discovery, localization, network, presentation) have no `json_types/` file; they use primitive-type helpers (`Firebolt::JSON::String`, `Boolean`, `Unsigned`) directly. `src/json_types/common.h` holds cross-module shared types and is not a module. - The singleton entry point is `Firebolt::IFireboltAccessor::Instance()`, implemented in `src/firebolt.cpp` as a Meyers singleton over `FireboltAccessorImpl`. **Anti-patterns:** @@ -44,23 +65,24 @@ Every new module must be wired into `src/firebolt.cpp`: 1. Member variable of type `Module::ModuleImpl` in `FireboltAccessorImpl`. 2. Initialised via `Firebolt::Helpers::GetHelperInstance()` in the constructor initialiser list. 3. Accessor method returning `Module::IModule&` override. -4. `unsubscribeAll()` call in the private `unsubscribeAll()` helper — **only if the module supports subscriptions** (i.e., the interface exposes any `subscribeOn*` methods). Note: `Device` currently exposes `subscribeOnHdrChanged(...)` but is absent from `FireboltAccessorImpl::unsubscribeAll()` — see Known Issue below. +4. `unsubscribeAll()` call in the private `unsubscribeAll()` helper — **only if the module supports subscriptions** (i.e., the interface exposes any `subscribeOn*` methods). -Reference: `src/firebolt.cpp` (`FireboltAccessorImpl` ctor initializer list and `unsubscribeAll()`). +Currently 10 modules are cleaned up there: accessibility, actions, device, lifecycle, localization, network, presentation, texttospeech, videooutput, speechsynthesis. Advertising, discovery, display, metrics, and stats have no subscriptions and are intentionally absent. When you add the first subscription to a module, add it to this helper in the same change. -**Known issue:** `FireboltAccessorImpl::unsubscribeAll()` in `src/firebolt.cpp` does not call `device_.unsubscribeAll()` even though `DeviceImpl` supports subscriptions (`subscribeOnHdrChanged`, `subscribeOnDolbyAtmosExperienceAvailableChanged`). Device subscriptions are never cleaned up on Disconnect. Track this as a separate defect. +Reference: `src/firebolt.cpp` (`FireboltAccessorImpl` ctor initializer list and `unsubscribeAll()`). ### 1.3 API-Facing Change Discipline -When making an API-facing change — adding or modifying an interface method, changing a return type, or adding a subscription — update all four artifacts in a single commit: +When making an API-facing change — adding or modifying an interface method, changing a return type, or adding a subscription — first check it against the applicable API specification (§0), then update all of these in a single commit: 1. `include/firebolt/.h` -2. `src/_impl.h` and `src/_impl.cpp` +2. `src/_impl.h` and `src/_impl.cpp` (and `src/json_types/.h` if the payload is a struct/enum) 3. `test/unit/Test.cpp` and `test/component/Test.cpp` -4. `docs/openrpc/the-spec/firebolt-open-rpc.json` — when component tests depend on fixture shape. +4. The relevant OpenRPC test fixture — method, schema, and examples used by component tests. Keep it consistent with the specification; it is not an API authority. +5. `test/api_test_app/apis/Demo.{h,cpp}` — where a demo exists for the module. Demos currently exist for accessibility, actions, advertising, device, discovery, display, lifecycle, localization, metrics, network, presentation, stats, texttospeech; none for speechsynthesis or videooutput. -Never leave the three layers out of sync after a commit. A compile-passing diff that omits a test update or fixture update is incomplete. +Never leave the layers out of sync after a commit. A compile-passing diff that omits a test update or fixture update is incomplete. -After the change, run `./run-component-tests-local.sh` to validate all layers together before pushing. +After the change, run `./run-unit-tests.sh`, `./run-component-tests-local.sh`, and `./lint.sh` (see §17) before pushing. --- @@ -103,17 +125,17 @@ After the change, run `./run-component-tests-local.sh` to validate all layers to - Getter/invoke RPC name: `"."` in camelCase (e.g., `"Device.chipsetId"`, `"Metrics.startContent"`). - Event name: `".on"` (e.g., `"Device.onHdrChanged"`, `"Actions.onIntent"`, `"Lifecycle2.onStateChanged"`). - `Lifecycle2` is the correct wire name for lifecycle RPCs in this version — do not use `Lifecycle`. -- TextToSpeech event names use lowercase suffixes matching the wire protocol (e.g., `"TextToSpeech.onWillspeak"`, `"TextToSpeech.onSpeechstart"`). These must match the OpenRPC fixture exactly. +- TextToSpeech event names are camelCase like every other module (`"TextToSpeech.onWillSpeak"`, `"TextToSpeech.onSpeechStart"`, `"TextToSpeech.onSpeechComplete"`). Two TextToSpeech *method* names differ in case from the specification: the client and fixture use `"TextToSpeech.listvoices"` and `"TextToSpeech.getspeechstate"`, while the specification names them `listVoices` and `getSpeechState` (§18, O-13). Do not change these wire strings without owner confirmation. **Anti-patterns:** -- Do not guess RPC or event string names. Always derive them from `docs/openrpc/the-spec/firebolt-open-rpc.json`. +- Do not guess RPC or event string names. Derive them from the applicable API specification; use fixtures and tests as verification only. ### 2.5 Enum Names **Current practice:** - C++ enum class names: `SCREAMING_SNAKE_CASE` (e.g., `INITIALIZING`, `ACTIVE`, `KILL_RELOAD`). - `EnumType` instance variable names: `Enum` (e.g., `LifecycleStateEnum`, `CloseReasonEnum`, `DeviceClassEnum`, `AgePolicyEnum`). -- Wire values in `EnumType` map: lowercase strings matching the OpenRPC fixture (e.g., `{"initializing", ...}`, `{"killReload", ...}`). +- Wire values in `EnumType` map: lowercase or camelCase strings as defined by the API specification (e.g., `{"initializing", ...}`, `{"killReload", ...}`). ### 2.6 JSON Adapter Classes @@ -126,7 +148,7 @@ After the change, run `./run-component-tests-local.sh` to validate all layers to **Current practice (partial inconsistency — see note):** - Unit test class: `UTest` inheriting `::testing::Test` and `MockBase` (e.g., `DeviceUTest`, `AccessibilityUTest`). - Component test class: `CTest` inheriting `::testing::Test` (e.g., `DeviceCTest`, `LifecycleCTest`). -- `ActionsGeneratedUTest` in `test/unit/actionsGeneratedTest.cpp` uses a non-standard name because it is auto-generated. This is acceptable only for auto-generated test files. +- Exceptions, all in files named `*GeneratedTest.cpp` (see §12): `ActionsGeneratedUTest`, `ActionsGeneratedCTest`, `VideooutputGeneratedUTest`, and `VideooutputGeneratedRuntimeCTest`. `ActionsUTest` (in `test/unit/actionsTest.cpp`) follows the standard pattern. The `*GeneratedUTest` classes do not inherit `MockBase`. **Recommended going forward:** New manually-authored test files must follow the `UTest` / `CTest` naming. @@ -137,12 +159,11 @@ After the change, run `./run-component-tests-local.sh` to validate all layers to ### 3.1 Header Guards **Current practice:** -- Bespoke headers (`include/firebolt/` and most `src/`): use `#pragma once`. -- Auto-generated **interface and impl** headers (e.g., `include/firebolt/actions.h`, `src/actions_impl.h`): use `#ifndef FIREBOLT__H` / `#define FIREBOLT__H` / `#endif` guards — confirmed in `include/firebolt/actions.h` and `src/actions_impl.h`. -- Auto-generated **json_types** headers (e.g., `src/json_types/actions.h`): use `#pragma once` — consistent with all bespoke json_types headers. -- Do not mix both guards in the same file. +- Most headers (13 of 15 modules' interface and impl headers, and most `json_types/`): use `#pragma once`. +- The `actions` and `videooutput` modules use `#ifndef FIREBOLT__H` / `#define` / `#endif` guards in their interface, impl, and json_types headers (e.g., `include/firebolt/actions.h`, `src/actions_impl.h`, `src/json_types/videooutput.h`). These two modules also have `*GeneratedTest.cpp` files; see §12 for provenance. +- Do not mix both guards in the same file. Two files currently do: `src/json_types/actions.h` and `src/videooutput_impl.h` carry both `#pragma once` and `#ifndef`. Cleanup: keep one. -**Recommended going forward:** All new bespoke headers use `#pragma once` exclusively. New auto-generated interface/impl headers follow the `#ifndef`/`#define`/`#endif` pattern; new auto-generated json_types headers follow `#pragma once`. +**Recommended going forward:** New headers use `#pragma once` exclusively. When editing an `actions`/`videooutput` header, keep its existing `#ifndef` guard rather than converting it. ### 3.2 Include Style @@ -197,7 +218,7 @@ private: ### 4.2 Deleted Copy Operations -**Current practice:** Copy constructor and copy assignment operator are explicitly deleted in **all** 13 `*Impl` classes and in `FireboltAccessorImpl` in `src/firebolt.cpp`. This is mandatory. +**Current practice:** Copy constructor and copy assignment operator are explicitly deleted in **all** 15 `*Impl` classes and in `FireboltAccessorImpl` in `src/firebolt.cpp`. This is mandatory. ### 4.3 Move Operations @@ -210,21 +231,7 @@ ClassName& operator=(ClassName&&) = delete; **Rationale:** These classes hold a non-reassignable reference member (`helper_`) and, where applicable, a `SubscriptionManager`. A compiler-generated move would leave the source object with a dangling reference or corrupted subscription state. Explicit deletion makes the intent clear and prevents accidental moves at call sites. -**Cleanup to fix:** No `*Impl` class currently deletes move operations. The following files must be updated: -- `src/accessibility_impl.h` -- `src/actions_impl.h` -- `src/advertising_impl.h` -- `src/device_impl.h` -- `src/discovery_impl.h` -- `src/display_impl.h` -- `src/lifecycle_impl.h` -- `src/localization_impl.h` -- `src/metrics_impl.h` -- `src/network_impl.h` -- `src/presentation_impl.h` -- `src/stats_impl.h` -- `src/texttospeech_impl.h` -- `src/firebolt.cpp` (`FireboltAccessorImpl`) +**Cleanup to fix:** None of the 15 `*Impl` classes nor `FireboltAccessorImpl` currently deletes move operations. Add the two deleted move declarations to every `src/*_impl.h` and to `FireboltAccessorImpl` in `src/firebolt.cpp`. New and modified `*Impl` classes must include them. ### 4.4 Destructor @@ -253,11 +260,13 @@ Do not use `friend` declarations to grant test classes access to implementation **Anti-pattern:** Do not throw exceptions from public interface methods. Do not return bare `T` where failure is possible. Do not use `std::optional` as a substitute for `Result` — `optional` cannot carry an error code. +**Constraint (no exceptions across the library boundary):** the library is consumed by a porting layer that may be built without C++ exceptions, so no exception may propagate out of any public method. Public signatures use standard API types (`std::string`, `uint32_t`, `std::optional<...>`) and do not expose parser-specific types such as `nlohmann::json`. For any entry point that parses caller-supplied text, catch the parse failure inside the impl and return `Error::InvalidParams`. Reference: `ActionsImpl::start()` in `src/actions_impl.cpp`. `[VERIFY]` the original design decision (recorded for RDKEMW-26413) with the library owners if you need to relax it. + ### 5.2 Error Propagation in Implementations -**Current practice:** Implementations propagate errors by returning the result of `helper_.get<>()`, `helper_.invoke()`, or `subscriptionManager_.subscribe<>()` directly. No intermediate `try-catch` is present in any `*_impl.cpp` file. +**Current practice:** Implementations propagate errors by returning the result of `helper_.get<>()`, `helper_.invoke()`, or `subscriptionManager_.subscribe<>()` directly. The only `try`/`catch` in any `*_impl.cpp` is in `ActionsImpl::start()`, which converts a `nlohmann::json::parse_error` on the caller's intent string into `Error::InvalidParams`. -**Anti-pattern:** Do not add `try-catch` blocks in `*_impl.cpp` for errors the helper/transport already handles. Do not swallow errors silently. +**Anti-pattern:** Do not add `try-catch` blocks in `*_impl.cpp` for errors the helper/transport already handles. A `try-catch` is permitted only to convert a failure while parsing **caller-supplied** input into `Result{Error::InvalidParams}`. Do not swallow errors silently. ### 5.3 Error Handling in JSON Adapters @@ -314,16 +323,21 @@ inline const Firebolt::JSON::EnumType<::> Enum({ ... }); ``` -Wire strings are lowercase or camelCase matching the OpenRPC fixture exactly. +Wire strings are lowercase or camelCase as defined by the API specification. Reference: `src/json_types/lifecycle.h` (`LifecycleStateEnum`, `CloseReasonEnum`), `src/json_types/device.h` (`DeviceClassEnum`), `src/json_types/common.h` (`AgePolicyEnum`). -### 6.4 Unit Conversion in JSON Adapters +### 6.4 Units in JSON Adapters (Stats) + +**Current practice:** `Stats.memoryUsage` carries `userMemoryUsed`, `userMemoryLimit`, `gpuMemoryUsed`, `gpuMemoryLimit` in **bytes**, as defined by the API specification. The current fixture and tests provide validation evidence. `src/json_types/stats.h` reads them as `uint64_t` with no conversion, and `Stats::MemoryInfo` exposes `uint64_t`. + +`[VERIFY]` The spec types these as 32-bit `unsigned`; the code uses `uint64_t`. See §18. + +**Anti-pattern:** Do not add unit conversion (e.g., ×1024) inside `fromJson()` without a matching change to the public API type, tests, fixture, and a spec confirmation. -**Current practice (specific to `Stats` module):** -The wire payload uses `*KiB` field names (e.g., `userMemoryUsedKiB`). The JSON adapter in `src/json_types/stats.h` reads the raw KiB values and the public API returns those values in KiB units. Tests in `test/unit/statsTest.cpp` and `test/component/statsTest.cpp` validate against the fixture's raw KiB values. +### 6.5 Raw JSON Payloads (Actions) -**Anti-pattern:** Do not add unit conversion (e.g., ×1024 for KiB→bytes) inside `fromJson()` without a corresponding change to the public API type, tests, and OpenRPC fixture annotation. +`Actions::Intent::intent` holds the intent as raw JSON **text**. `JsonData::JsonValue::fromJson()` stores `json["intent"].dump()`. `nlohmann::json` never appears in a public header. Because `dump()` may reorder object keys, tests must compare structurally (`nlohmann::json::parse(result->intent) == expected`), never by raw string equality. See §12.4. --- @@ -347,7 +361,7 @@ Reference: `src/device_impl.cpp`, `src/localization_impl.cpp`. ### 7.3 `helper_.invoke(methodName, params)` — Fire-and-Forget Methods -**Current practice:** Used for methods that return `Result`. Parameters are constructed as `nlohmann::json` before the call. Optional parameters are conditionally added. +**Current practice:** Used for methods that return `Result`. Parameters are constructed as `nlohmann::json` before the call. Optional parameters are conditionally added. Request parameter names are written as literals in the impl and must match the spec's parameter names. Response parsing never happens in the impl; it lives in `json_types/`. Reference: `src/metrics_impl.cpp` (all methods), `src/lifecycle_impl.cpp` (`close()`). @@ -372,7 +386,7 @@ Reference: `src/accessibility_impl.cpp`, `src/actions_impl.cpp`, `src/lifecycle_ ### 8.2 Component Tests -**Current practice (confirmed in all event-bearing component tests):** +**Current practice (the fixture style below is the most common, but not universal):** ```cpp class ModuleCTest : public ::testing::Test { @@ -385,10 +399,13 @@ protected: ``` Event delivery uses `cv.wait_for(lock, EventWaitTime, [&] { return eventReceived; })` via `verifyEventReceived()` and `verifyEventNotReceived()` from `test/utils.h`. `EventWaitTime` is `std::chrono::seconds(2)` (defined in `test/utils.cpp`). +Variations that exist today: `TextToSpeechCTest` declares `cv`/`mtx`/`eventReceived` as locals inside each test; `SpeechSynthesisCTest` uses members named `mtx_`, `cv_`, `eventReceived_`; `MetricsCTest::SetUp()` is empty. Keep the style of the file you are editing. + **Current practice — triggering events:** - String payload: `triggerEvent("Module.onEvent", R"("string_value")")` — note outer double-quotes in JSON - Object payload: `triggerEvent("Module.onEvent", R"({"field": value})")` -- For `Actions.onIntent`, the payload is a JSON-encoded object: `triggerEvent("Actions.onIntent", R"({"intent":"launch","intentId":1})")`. The callback receives this string and must parse it with `nlohmann::json::parse()`. See §12.4 for the full contract. +- Scalar events: two forms are used today. Wrapped: `R"({"value": true})"` (Accessibility, Network, Presentation, Localization, `Device.onDolbyAtmosExperienceAvailableChanged`). Bare: `R"("Living Room TV")"` (`Device.onNameChanged`) and `R"("hdcp1.4")"` (VideoOutput). `[VERIFY]` which form is canonical for mock-firebolt (§18). Copy the form used by the nearest existing test for the same payload type. +- For `Actions.onIntent`, the payload is an object whose `intent` member is itself a JSON **object**: `R"({"intent":{"action":"pre-load",...},"intentId":0})"`. The callback receives `Actions::Intent` with `intent` as raw JSON text. See §12.4 for the full contract. Reference: `test/component/actionsGeneratedTest.cpp`, `test/component/deviceTest.cpp`, `test/component/networkTest.cpp`. @@ -418,12 +435,15 @@ Reference: `test/component/actionsGeneratedTest.cpp`, `test/component/deviceTest **Test case rules (confirmed across all unit test files):** - Happy path getter: call `mock("Module.method")`, then call impl method, then `ASSERT_TRUE(result)` + value check. - Negative path (bad wire data): call `mock_with_response("Module.method", )`, then `ASSERT_FALSE(result)`. +- Invoke-style (`Result`) method: call `mockInvoke("Module.method")`, then the impl method, then `ASSERT_TRUE(result)`. - Subscribe test: call `mockSubscribe("Module.onEvent")`, subscribe, assert `ASSERT_TRUE(result)`, then call `unsubscribe` and assert success. - Enum validation: `validate_enum("EnumName", Firebolt::::JsonData::Enum)` checks the fixture's schema against the code's enum map. **Anti-pattern:** Do not test `*Impl` via `IFireboltAccessor::Instance()` in unit tests. Unit tests must isolate the impl with a mock helper, not the full singleton. -**Exception for auto-generated unit tests:** `test/unit/actionsGeneratedTest.cpp` directly instantiates `::testing::NiceMock` without inheriting `MockBase`, and uses `EXPECT_CALL(mockHelper, getJson(...))` rather than the `mock()` / `mock_with_response()` convenience wrappers. This pattern is generator-owned. Do not replicate it in bespoke test files. +**Exception for `*GeneratedTest.cpp` unit tests:** `test/unit/actionsGeneratedTest.cpp` and `test/unit/videooutputGeneratedTest.cpp` instantiate `::testing::NiceMock` directly without inheriting `MockBase`, and use `EXPECT_CALL(mockHelper, getJson(...))` rather than the `mock()` / `mock_with_response()` wrappers. Do not replicate this in new bespoke test files. + +Bespoke tests (`discoveryTest.cpp`, `speechSynthesisTest.cpp`, `textToSpeechTest.cpp`) do use `EXPECT_CALL(mockHelper, getJson/invoke(...))` alongside `MockBase` when they need to assert the exact request payload. That is acceptable for payload assertions. ### 10.2 Component Tests @@ -447,9 +467,9 @@ Reference: `test/component/actionsGeneratedTest.cpp`, `test/component/deviceTest Reference: `test/component/lifecycleTest.cpp` (`subscribeOnState_JSON_RPC_compliant`). -**Component test log expectations:** Red schema validation lines in the component test log are expected and normal for negative-path tests — they indicate the transport rejected the invalid payload as intended. Do not treat them as test failures and do not suppress them by weakening the test. +**Component test log expectations:** mock-firebolt prints lines of the form ` validation error for with , err: [...]` when a negative-path test injects an invalid payload. This is expected: the mock rejects the payload (event validation is enabled in `.github/mock-firebolt/config.json`), so the client never receives it. Do not treat these lines as failures and do not suppress them by weakening the test. -Negative tests must verify runtime behaviour — specifically that callbacks are not delivered when the payload is invalid. A test that merely asserts the code compiles with an invalid type is insufficient. Always pair with `verifyEventNotReceived`. Do not relax or remove a negative test because it produces red schema validation lines. +Negative tests must verify runtime behaviour — specifically that callbacks are not delivered when the payload is invalid. A test that merely asserts the code compiles with an invalid type is insufficient. Always pair with `verifyEventNotReceived`. Do not relax or remove a negative test because it produces validation-error log lines. ### 10.3 Pairing Rule @@ -457,13 +477,13 @@ Negative tests must verify runtime behaviour — specifically that callbacks are - Add unit tests in `test/unit/Test.cpp` - Add component tests in `test/component/Test.cpp` - Both test files must cover all public API methods -- Each getter/property method must have at minimum: one happy-path test and one bad-response negative test +- Each getter/property method must have at minimum: one happy-path test and one bad-response negative test. This is required for new and modified code; existing coverage is not uniformly complete `[VERIFY]`. ### 10.4 Expected Values from OpenRPC **Current practice:** Both unit and component tests derive expected values from `jsonEngine.get_value("Module.method")` (the first example in the OpenRPC fixture). Do not hardcode values that duplicate the fixture unless the value requires a type conversion (e.g., enum comparison using `static_cast`). -Exception: `test/component/actionsGeneratedTest.cpp` hardcodes `"launch"` (the `intent` field) and `1` (the `intentId` field) from the fixture's `Actions.intent` / `Actions.onIntent` example result — permissible only for auto-generated files. +Exception: `test/component/actionsGeneratedTest.cpp` hardcodes the intent payload (`{"action":"pre-load",...}`, `intentId` 0) copied from the fixture's `Actions.intent` / `Actions.onIntent` example. If the fixture example changes, update that test in the same commit. --- @@ -472,8 +492,8 @@ Exception: `test/component/actionsGeneratedTest.cpp` hardcodes `"launch"` (the ` ### 11.1 Fixture Examples and Enum Alignment **Current practice:** -- Fixture location: `docs/openrpc/the-spec/firebolt-open-rpc.json` -- Both unit and component test binaries read this file at runtime (path injected via `UT_OPEN_RPC_FILE` define in `test/CMakeLists.txt`). +- Fixture location: `docs/openrpc/the-spec/firebolt-open-rpc.json`. +- Both unit and component test binaries read this file at runtime (path injected via the `UT_OPEN_RPC_FILE` define in `test/CMakeLists.txt`). - When adding or changing a method, the fixture must be updated to include the method, its parameters schema, and at least one example. - Enum values in code must match `components.schemas..enum` in the fixture — validated by `validate_enum()`. @@ -487,60 +507,54 @@ Exception: `test/component/actionsGeneratedTest.cpp` hardcodes `"launch"` (the ` **Notifier/subscriber metadata:** Subscription event entries must keep `x-notifier` and `x-subscriber-for` fields aligned with the corresponding getter or property. Adding a subscription event without updating both fields is a fixture defect. +### 11.3 Event Payload Validation in mock-firebolt + +- `.github/mock-firebolt/config.json` enables validation of `method`, `params`, `response`, and `events`. A `triggerEvent` payload that fails validation is dropped, and the test fails with `Did not receive event within timeout`. +- Observed rule (recorded 2026-10-05 for the TextToSpeech events): for an event **with** `x-subscriber-for`, mock-firebolt validates the payload against the linked getter's result schema, so the event's own `result` may be `{"type": "null"}`. For a pure notifier **without** `x-subscriber-for` (all `TextToSpeech.on*`, `SpeechSynthesis.on*`), it validates against the event's own `result` schema, which must therefore describe the real payload (e.g. `$ref: SpeechIdEvent`). +- `[VERIFY]` `Lifecycle2.onStateChanged` is a pure notifier whose fixture `result` is `{"type": "null"}`, yet its component tests deliver payloads successfully. The rule above is therefore incomplete; confirm the exact behaviour against the pinned mock-firebolt revision before relying on it. +- The mock-firebolt branch and commit used by CI are defined in `.github/workflows/L2-tests.yml`; local component tests pin the same values in `run-component-tests-local.sh`. Keep these two pins identical. +- With `--protocol legacy`, `.github/scripts/run-component-tests.sh` switches to `*--legacy.json` variants of the mock config and the OpenRPC fixture. `[VERIFY]` those variants exist in the repo before relying on the legacy path. + --- -## 12. Auto-Generated vs Bespoke Code +## 12. Possibly Generated vs Bespoke Code -### 12.1 Auto-Generated File Recognition +### 12.1 Possibly Generated Files (Provenance Unconfirmed) -Files with the following banner are owned by the `firebolt-sdk-gen` generator tool — do not modify them directly: -``` -// ============================================================================ -// AUTO-GENERATED by fb-gen — DO NOT EDIT -// ============================================================================ -``` +No file in this repository carries an `AUTO-GENERATED` / `DO NOT EDIT` banner, and neither `firebolt-sdk-gen` nor `sync-plan-checklist.sh` exists in the repo or in the sibling checkouts under `RDKE/`. An earlier version of this document declared `actions` files generator-owned; that cannot be confirmed. -Confirmed auto-generated files in the repo: -- `include/firebolt/actions.h` -- `src/actions_impl.h` -- `src/actions_impl.cpp` -- `src/json_types/actions.h` -- `test/unit/actionsGeneratedTest.cpp` -- `test/component/actionsGeneratedTest.cpp` +Two modules differ stylistically from the rest and are *possibly* generator-originated: `actions` and `videooutput`. They use `#ifndef` guards (§3.1), their tests are named `*GeneratedTest.cpp` (§2.7, §10.1), and their JSON adapters use a `*Json` / `*Value` naming style. Files: +- `include/firebolt/actions.h`, `src/actions_impl.{h,cpp}`, `src/json_types/actions.h`, `test/unit/actionsGeneratedTest.cpp`, `test/component/actionsGeneratedTest.cpp` +- `include/firebolt/videooutput.h`, `src/videooutput_impl.{h,cpp}`, `src/json_types/videooutput.h`, `test/unit/videooutputGeneratedTest.cpp`, `test/component/videooutputGeneratedTest.cpp` -### 12.2 Modifying Auto-Generated Output +These files have been hand-edited in this repository (for example `ActionsImpl::start()` and `JsonData::JsonValue` for RDKEMW-26413), so "never hand-edit" is not the practice. -When a change is generator-owned, use `firebolt-sdk-gen` from the sibling repo: -```bash -./sync-plan-checklist.sh --profile core --module --apply --no-accessor-touchpoints --target-root ../firebolt-cpp-client -``` -Do not hand-edit auto-generated files. If the generated output has a defect, fix the generator. +### 12.2 Editing These Files -### 12.3 Keeping Bespoke and Generated Files Aligned +- Hand edits are allowed. Keep each file's existing style (guards, naming) so a future generator run produces a small diff. +- Do not perform large structural rewrites of these files until provenance is confirmed (§18, O-9). +- If a generator is later identified, record its location, command, and ownership here and restore a DO-NOT-EDIT rule only for files that actually carry a banner. -When a new bespoke module is added, ensure it follows the same structure as generated modules (`actions`) so the two styles remain similar enough that the generator could own the bespoke code in the future. +### 12.3 Structure of New Modules -### 12.4 Actions Module API Contract +New modules follow the structure of the `#pragma once` modules (for example `speechsynthesis`, `device`), not the `actions`/`videooutput` style, unless a generator is confirmed to own the module. -`Actions.intent` is a getter-only method: it takes no parameters and returns `Result`. Do not add parameters to it and do not change its return type. +### 12.4 Actions Module API Contract -The `std::string` returned by `Actions.intent()` is a JSON-serialized object, not a plain scalar. Callers must parse it (confirmed in `test/component/actionsGeneratedTest.cpp`): -```cpp -auto result = accessor.ActionsInterface().intent(); -ASSERT_TRUE(result); -auto parsed = nlohmann::json::parse(*result); -EXPECT_EQ(parsed.at("intent").get(), "launch"); -EXPECT_EQ(parsed.at("intentId").get(), 1); -``` -Do not treat the return value as a plain scalar string. +The API specification defines `start`, `intent`, and `onIntent` for API 9.0.0. `intentId` is an unsigned, monotonic value; `intent` is a JSON document. -The `Actions.onIntent` callback also delivers a JSON-serialized object string. The component event trigger must use a JSON-encoded object payload (confirmed at `test/component/actionsGeneratedTest.cpp:62`): +- `IActions::intent()` is a getter with no parameters. It returns `Result`, where `struct Intent { std::string intent; uint32_t intentId{0}; }`. `intent` holds **raw JSON text**; it is not a plain scalar and not an `nlohmann::json`. +- `subscribeOnIntent(std::function&&)` delivers the same struct. `subscribeOnIntentChanged` is a default alias that forwards to it. +- `start(const std::string& intent, std::optional handlerAppId = std::nullopt)` returns `Result`. A malformed `intent` string returns `Error::InvalidParams`; it must never throw (§5.1). +- Wire shape (fixture): `{"intent": {"action": "pre-load", ...}, "intentId": 0}` — `intent` is a JSON object on the wire. +- Tests compare structurally, not by string equality (`dump()` may reorder keys). Reference: `test/component/actionsGeneratedTest.cpp`: ```cpp -triggerEvent("Actions.onIntent", R"({"intent":"launch","intentId":1})") +auto result = Firebolt::IFireboltAccessor::Instance().ActionsInterface().intent(); +ASSERT_TRUE(result) << toError(result); +EXPECT_EQ(nlohmann::json::parse(result->intent), expectedIntent); +EXPECT_EQ(result->intentId, 0U); ``` -The callback receives the full JSON string and must parse it with `nlohmann::json::parse(intent)`. Do not use a plain string payload such as `R"("launch")"`. - -The OpenRPC fixture confirms: both `Actions.intent` and `Actions.onIntent` example results are `{"intent": "launch", "intentId": 1}` (verified in `docs/openrpc/the-spec/firebolt-open-rpc.json`). +Do not add `nlohmann` types to `include/firebolt/actions.h`, and do not compare `intent` text with `==`. --- @@ -551,13 +565,16 @@ The OpenRPC fixture confirms: both `Actions.intent` and `Actions.onIntent` examp - Warning flags: `-Wall -Wextra -Wpedantic` are unconditionally applied in `CMakeLists.txt`. - New `*_impl.cpp` files are picked up automatically via `file(GLOB SOURCES CONFIGURE_DEPENDS *.cpp json_types/*.cpp)` in `src/CMakeLists.txt`. - New test files are picked up automatically via `file(GLOB UNIT_TESTS CONFIGURE_DEPENDS unit/*Test.cpp)` and `file(GLOB COMPONENT_TESTS CONFIGURE_DEPENDS component/*Test.cpp)`. -- Export macro: `FIREBOLTCLIENT_EXPORT` from the generated `firebolt/client_export.h`. Apply to public symbols in `include/firebolt/firebolt.h`. +- Export macro: `FIREBOLTCLIENT_EXPORT` from the generated `firebolt/client_export.h`; it is applied to `IFireboltAccessor` in `include/firebolt/firebolt.h`. Symbol visibility is `hidden` unless `ENABLE_TESTS` is on, so only exported symbols are usable from outside the library in a normal build. Do not reference `*Impl` classes from the demo app or other consumers. +- Transport dependency: `.transport.version` pins the expected `firebolt-cpp-transport`; the root `CMakeLists.txt` compares major versions only and reports a mismatch via `message(STATUS ...)`. **Anti-pattern:** Do not manually list sources in `src/CMakeLists.txt` — the glob handles this. Do not introduce new `CMakeLists.txt` files inside nested subdirectories under `src/` or `test/` (e.g., `src/json_types/`, `test/unit/`, `test/component/`). The top-level `src/CMakeLists.txt` and `test/CMakeLists.txt` already exist and must not be replaced. -**Formatting enforced by CI:** `clang-format` with the project's `.clang-format` (LLVM-based, column limit 120, 4-space indent, Allman braces, C++17). Running `git ls-files -- '*.cpp' '*.h' | xargs clang-format --dry-run --Werror` is a required CI check. Do not bypass it. +**Formatting enforced by CI:** `clang-format` with the project's `.clang-format` (LLVM-based, column limit 120, 4-space indent, Allman braces, C++17). The `format_check` job in `.github/workflows/tests-trigger.yml` runs `git ls-files -- '*.cpp' '*.h' | xargs clang-format --dry-run --Werror`. Do not bypass it. Locally use `./lint.sh` (`--fix` to reformat); see §17 for a caveat that can make it check zero files. + +**CI compatibility:** All build and source changes must remain compatible with the CI workflows. Do not modify build configuration in a way that passes locally but diverges from the Docker-based environment defined by `.github/Dockerfile`, `.github/workflows/tests-trigger.yml` (jobs: `build_docker`, `format_check`, `build_project`, `coverage_gate`, `update_baseline`), `.github/workflows/L1-tests.yml` (unit tests and coverage), `.github/workflows/L2-tests.yml` (component tests and `api_test_app`), and `.github/scripts/run-component-tests.sh`. -**CI compatibility:** All build and source changes must remain compatible with the CI workflow. Do not modify build configuration in a way that passes locally but diverges from the Docker-based environment defined in `.github/workflows/ci.yml` and `.github/scripts/run-component-tests.sh`. +**Coverage gate:** `coverage_gate` compares unit and component coverage against a baseline (`.github/scripts/compare_coverage.py`). New or changed code must come with tests (§10.3) or the gate can fail. --- @@ -573,14 +590,16 @@ The following patterns are explicitly wrong for this codebase. Each entry notes | AP-4 | Storing `IHelper` by pointer | Consistent reference storage; pointer would allow null and is not the established contract | | AP-5 | Making `*Impl` classes copyable or movable | They hold a non-owning reference (`helper_`) and a `SubscriptionManager`; copying/moving would silently break subscription ownership tracking | | AP-6 | Calling `IFireboltAccessor::Instance()` in unit tests | Unit tests must isolate the impl with `MockHelper`; the singleton instantiates real transport | -| AP-7 | Hardcoding JSON field names as magic strings in `*_impl.cpp` | Field names must live in `src/json_types/` only; impl code must not parse JSON directly | +| AP-7 | Parsing **response** JSON in `*_impl.cpp`, or hardcoding response field names there | Response deserialization lives in `src/json_types/` only. Building **request** parameter objects with literal names in the impl is normal and required (names must match the spec). The sole parse in an impl is `ActionsImpl::start()` validating caller input (§5.2) | | AP-8 | Adding `nlohmann::json` includes to `include/firebolt/*.h` | Public headers must not expose the JSON library as a transitive dependency | | AP-9 | Adding logging to `*_impl.cpp` | Logging is intentionally absent in module implementations; all diagnostics go through the transport layer | | AP-10 | Writing a new module that omits `subscribeOnStateChanged`-style subscription when the OpenRPC spec has `on*` events | Subscriptions are load-bearing API surface; omitting them silently breaks consumer event handling | -| AP-11 | Assuming `Actions.onIntent` uses a plain string trigger payload | The actual payload is a JSON-encoded object: `triggerEvent("Actions.onIntent", R"({"intent":"launch","intentId":1})")` (confirmed at `test/component/actionsGeneratedTest.cpp:62`). Always check each module's component test file and the OpenRPC fixture for the correct payload shape before writing event trigger calls | +| AP-11 | Copying an event payload from another module without checking its shape | Shapes differ: wrapped scalars (`{"value": true}`), bare scalars (`"hdcp1.4"`), objects (`{"speechid": 1}`), and `Actions.onIntent` (`{"intent": {...}, "intentId": 0}`, with `intent` an object). Check the module's component test and the fixture before writing `triggerEvent` calls (§8.2) | | AP-12 | Using `#include ` or other heavyweight headers without a direct use | Unnecessary includes increase compile time and leak transitive dependencies into consumers' include graphs. `-Wall -Wextra -Wpedantic` do not warn on unused includes; keep includes minimal as a discipline, not for warning suppression. `` is the canonical example of a heavyweight header with no use in this codebase | -| AP-13 | Defining a new public header without `#pragma once` | Bespoke headers require `#pragma once`; `#ifndef` guards are reserved for auto-generated output | -| AP-14 | Modifying auto-generated files by hand | Files with `// AUTO-GENERATED by fb-gen — DO NOT EDIT` must be regenerated via the generator tool | +| AP-13 | Defining a new header without `#pragma once` | New headers use `#pragma once`; `#ifndef` guards exist only in the `actions` and `videooutput` modules (§3.1) | +| AP-14 | Large structural rewrites of `actions` / `videooutput` files before provenance is confirmed | They may be generator-originated (§12.1); small targeted edits are fine and have precedent | +| AP-15 | Treating `./lint.sh` printing "Formatting OK" as proof of a clean format on macOS/Docker | The script pipes `git ls-files` into `xargs` without `pipefail`; if Git rejects the repo as "dubious ownership", zero files are checked. Confirm the file count (§17) | +| AP-16 | Regenerating `docs/openrpc/the-spec/*.json` (`npm run the-spec`) and committing without diffing | The per-module sources under `docs/openrpc/openrpc/` use different TextToSpeech event names (`onWillspeak`) than the merged fixture and client (`onWillSpeak`). Blind regeneration can break component tests (§18, O-5) | --- @@ -590,10 +609,10 @@ The following items are inferred from code patterns where no explicit policy doc | ID | Assumption | |---|---| -| A-1 | `FIREBOLT_LOG_*` macros come from `FireboltTransport`. Individual module impls intentionally omit logging — inferred from the absence of any logging in 12 of 13 impl files. | -| A-2 | The `using namespace Firebolt::Helpers;` pattern in `stats_impl.cpp` and `lifecycle_impl.cpp` is incidental rather than policy — inferred from its absence in the other 11 impl files. | -| A-3 | `StatsImpl`'s, `LifecycleImpl`'s, and `LocalizationImpl`'s non-`explicit` constructors are legacy remnants — confirmed at `src/stats_impl.h:30`, `src/lifecycle_impl.h:37`, `src/localization_impl.h:29`. `StatsImpl`'s non-`= default` destructor is also a remnant. All other impls use `explicit` and `= default`. | -| A-4 | Wire names for TextToSpeech events (`onWillspeak`, `onSpeechstart`, etc.) are lowercase-concatenated because the Firebolt protocol lowercases them — inferred from the pattern in `src/texttospeech_impl.cpp` and the absence of a different naming convention for other modules' events. | +| A-1 | `FIREBOLT_LOG_*` macros come from `FireboltTransport`. Individual module impls intentionally omit logging — inferred from the absence of any logging in all 15 `*_impl.cpp` files. | +| A-2 | The `using namespace Firebolt::Helpers;` pattern in `stats_impl.cpp` and `lifecycle_impl.cpp` is incidental rather than policy — inferred from its absence in the other 13 impl files. | +| A-3 | The non-`explicit` constructors of `StatsImpl`, `LifecycleImpl`, and `LocalizationImpl`, and the out-of-line empty destructors of `StatsImpl` and `LifecycleImpl`, are legacy remnants. All other impls use `explicit` and `= default`. Locations: `src/stats_impl.h`, `src/lifecycle_impl.h`, `src/localization_impl.h`. | +| A-4 | TextToSpeech event wire names are camelCase (`onWillSpeak`, `onSpeechStart`, ...) as used in `src/texttospeech_impl.cpp` and `the-spec/firebolt-open-rpc.json`. Commit `89cb7fc` ("Fix TTS event wire names to use camelCase") corrected earlier lowercase names. The per-module source `docs/openrpc/openrpc/text_to_speech.json` still has lowercase names — see §18, O-5. | --- @@ -607,15 +626,50 @@ This document is the single authoritative source for coding conventions and work ## 17. Test Execution Commands -**Component tests (current preferred, local):** ```bash -./run-component-tests-local.sh +./run-unit-tests.sh # builds build-dev (+tests), runs utApp +./run-unit-tests.sh --unit-filter "ActionsUTest.*" +./run-component-tests-local.sh # Docker: builds ctApp in build-docker, runs against mock-firebolt ./run-component-tests-local.sh --skip-image-build # reuse existing Docker image +./lint.sh # clang-format check; --fix to reformat; SKIP_DOCKER=1 or --local for host clang-format ``` -**Unit tests only:** +Always run component tests after any API-facing change. Component tests run in Docker against mock-firebolt and are the authoritative validation gate. Run unit tests, component tests, and lint before pushing. + +### Known environment pitfalls (macOS + Docker Desktop) + +Verified on 2026-10-14. These are environment issues, not code defects; do not "fix" them by editing source. + +- **`./run-unit-tests.sh` fails with "CMakeCache.txt directory is different than ... /workspace/build-dev"** when `build-dev` was previously built inside Docker (its cache records `/workspace`). Use `./run-unit-tests.sh --clean` (removes `build-dev`) or build in a fresh directory. +- **`./run-component-tests-local.sh` fails with "detected dubious ownership in repository at '/mock-host'"** when it clones/updates mock-firebolt in the container. It also deletes and recreates `build-docker` on every run. +- **`./lint.sh` can print "Formatting OK" while checking nothing.** The container's Git prints "dubious ownership ... '/workspace'", `git ls-files` returns nothing useful, and the unguarded pipe lets `xargs clang-format` succeed on empty input. Observed: 4 lines vs. 126 tracked `*.cpp`/`*.h` files once the repo is trusted. Verify the real check with: ```bash -./run-unit-tests.sh +docker run --rm --user "$(id -u):$(id -g)" -e GIT_CONFIG_COUNT=1 -e GIT_CONFIG_KEY_0=safe.directory -e GIT_CONFIG_VALUE_0=/workspace \ + -v "$PWD:/workspace" -w /workspace firebolt-client-ci:local \ + bash -c "git ls-files -- '*.cpp' '*.h' | xargs clang-format --dry-run --Werror" ``` +- **Running components manually** inside `firebolt-client-ci:local` additionally needs `-e PATH=/usr/local/nvm/versions/node/v24.11.0/bin:$PATH` and `-e npm_config_cache=/tmp/npm-cache` for `npm ci` in mock-firebolt (the image's default `PATH` has no `node`/`npm`, and `/.npm` is root-owned), plus the same `GIT_CONFIG_*` safe-directory variables for `/workspace` and `/mock`. + +Proposed script fixes (not yet applied): add `set -o pipefail` and a safe-directory setting to `lint.sh`; add the same safe-directory setting and an `npm` cache/`PATH` to `run-component-tests-local.sh`. + +--- -Always run component tests after any API-facing change. Component tests run in Docker against mock-firebolt and are the authoritative validation gate. +## 18. Open Items Requiring Investigation + +Each item is unverified or unresolved. Do not "fix" these by assumption; confirm with the owner or evidence, then update this section and the relevant rule above. + +| ID | Item | Evidence | Needed | +|---|---|---|---| +| O-1 | Spec methods marked `C++17: ✅` that the client does not implement | Device: `manufacturer`, `brandName`, `modelId`. Display: `connected` / `onConnectedChanged`. Network: `interfaces`, `interfaceStatus`/`onInterfaceStatusChanged`, `interfaceConfig`/`onInterfaceConfigChanged`, `interfaceStatistics`, `interfaceDetails`/`onInterfaceDetailsChanged` (none in `include/firebolt/*.h`) | Confirm roadmap and ticket owner; these are gaps, not intentional exclusions | +| O-2 | Client methods that are not in the spec | `Device.setOsName`, `Device.setOsVersion`; `Discovery.watchedV2`; extra optional `TextToSpeech.speak` parameters (`callSign`, `language`, `voice`, `volume`, `rate`, `pitch`) appear in the fixture but spec #73 lists only `text` | Confirm they are intended extensions or align to the spec | +| O-3 | `Discovery.watched` shape | Spec #22 returns None. `watched()` returns `Result` (fixture agrees); `watchedV2()` returns `Result` but invokes wire method `"Discovery.watched"` (unit tests assert this), while the fixture defines a separate `Discovery.watchedV2` returning null | Decide the canonical wire method and return type | +| O-4 | Stats type width | Spec #72 says 32-bit `unsigned`; `Stats::MemoryInfo` and `json_types/stats.h` use `uint64_t` (bytes, no conversion) | Confirm `uint64_t` is intended; update spec or code | +| O-5 | TextToSpeech event names diverge between OpenRPC sources | Per-module `docs/openrpc/openrpc/text_to_speech.json` uses `onWillspeak`/`onSpeechstart`/... (`x-event`); the tracked merged fixture `docs/openrpc/the-spec/firebolt-open-rpc.json` and the client use `onWillSpeak`/`onSpeechStart`/... The merged fixture appears to be edited directly | Define the official regeneration process (`npm run the-spec` in `docs/openrpc`), fix the sources, or document that the merged file is hand-maintained | +| O-6 | Event validation rule in mock-firebolt (§11.3) is incomplete | `Lifecycle2.onStateChanged` has a `null` result schema and no `x-subscriber-for`, yet its component tests deliver payloads | Confirm validation behaviour at the pinned mock revision | +| O-7 | Canonical scalar event payload form | Both `{"value": x}` and bare `x` are used in component tests | Confirm what the mock/gateway emit on the real wire | +| O-8 | Spec error semantics | Spec lists codes (-32601, -40300, -32602, -32100, -50100) and unassigned `-xxxxx` for *App state invalid*, *Speech synthesis engine error*, *Wrong device class* | Verify how these map to `Firebolt::Error` (defined in the transport dependency, not in this repo) | +| O-9 | Provenance of `actions` / `videooutput` files | No generator banner; `firebolt-sdk-gen` and `sync-plan-checklist.sh` not found (§12.1) | Identify the generator, if any, and its ownership rules | +| O-10 | Lifecycle state set | Spec #31 lists six states with no `unloaded`; the current enum matches the spec. A backend `unloaded` response was handled on a separate branch (RDKEMW-26354) | Confirm whether `unloaded` is a real wire value and update the spec/enum together | +| O-11 | Open cleanups from §4 and §3.1 | Deleted moves everywhere; `explicit` on three impls; empty destructors on two; `u_int32_t` in `src/device_impl.h`; `friend class ::LifecycleTest` in `src/lifecycle_impl.h`; `using namespace` in two `.cpp` files; double guards in `src/json_types/actions.h` and `src/videooutput_impl.h` | Schedule a single cleanup change | +| O-12 | Script defects in §17 | `lint.sh` no `pipefail`/safe-directory; `run-component-tests-local.sh` safe-directory, `npm`/`PATH`, and cache handling. Whether CI's `format_check` is exposed to the same ownership problem is unverified | Fix scripts; confirm CI behaviour | +| O-13 | TextToSpeech method names differ in case from the specification | The specification names `getSpeechState` (#77) and `listVoices` (#86); `src/texttospeech_impl.cpp` and the fixture use `listvoices` and `getspeechstate`. The module is deprecated at API 9.0.0 | Confirm the lowercase wire names are intended for the backend, then align the specification or the client and fixture together | diff --git a/.github/scripts/compare_coverage_test.py b/.github/scripts/compare_coverage_test.py index 73e956f..2052fce 100644 --- a/.github/scripts/compare_coverage_test.py +++ b/.github/scripts/compare_coverage_test.py @@ -22,7 +22,7 @@ - Integration tests (subprocess) simulating all gate scenarios listed in the Coverage Gate implementation spec. -Workflow-level scenarios (unit_tests fails / component_tests fails / both fail) are handled +Workflow-level scenarios (L1 fails / L2 fails / both fail) are handled by GitHub Actions' implicit success() dependency check on the coverage-gate job and cannot be tested at the Python script level; they are documented inline. """ @@ -628,12 +628,12 @@ def test_s9_non_dict_json(self): self.assertEqual(r.returncode, 0, msg=r.stdout + r.stderr) # =========================================================================== - # SCENARIO 10 — unit_tests job fails + # SCENARIO 10 — L1 (unit) job fails # Coverage Gate does NOT trigger (workflow-level behaviour). # - # GitHub Actions: coverage_gate has `needs: [unit_tests, component_tests]` + # GitHub Actions: coverage_gate has `needs: [trigger-L1, trigger-L2]` # with no custom `if:`. The implicit success() check means coverage_gate - # is SKIPPED whenever unit_tests fails. This cannot be unit-tested here; + # is SKIPPED whenever trigger-L1 fails. This cannot be unit-tested here; # it is enforced by the workflow graph. # =========================================================================== @@ -652,7 +652,7 @@ def test_s10_unit_artifacts_absent_component_passes(self): self.assertIn("[WARN]", r.stdout) # =========================================================================== - # SCENARIO 11 — component_tests job fails (symmetric to scenario 10) + # SCENARIO 11 — L2 (component) job fails (symmetric to scenario 10) # =========================================================================== def test_s11_component_artifacts_absent_unit_passes(self): @@ -664,7 +664,7 @@ def test_s11_component_artifacts_absent_unit_passes(self): self.assertIn("[WARN]", r.stdout) # =========================================================================== - # SCENARIO 12 — Both unit_tests AND component_tests jobs fail + # SCENARIO 12 — Both L1 AND L2 jobs fail # Coverage Gate does NOT trigger (workflow-level). At script level, both # .info files are absent → both SKIP → exit 0 (harmless; gate is already # blocked at the workflow graph layer before the script is ever called). diff --git a/.github/workflows/L1-tests.yml b/.github/workflows/L1-tests.yml new file mode 100644 index 0000000..2e624a4 --- /dev/null +++ b/.github/workflows/L1-tests.yml @@ -0,0 +1,81 @@ +name: L1-tests + +on: + workflow_call: + inputs: + image_tag: + required: true + type: string + +permissions: + contents: read + packages: read + +defaults: + run: + shell: bash + +jobs: + L1-tests: + name: Run unit tests and collect coverage + runs-on: ubuntu-latest + steps: + - name: Log in to the Container registry + uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Checkout Code + uses: actions/checkout@v4 + + - name: Download build directory + uses: actions/download-artifact@v4 + with: + name: build-dir + path: ${{ github.workspace }}/build + + - name: Run Unit Tests + run: | + chmod +x ${{ github.workspace }}/build/test/utApp + docker run --rm --user "$(id -u):$(id -g)" -v ${{ github.workspace }}:/workspace ${{ inputs.image_tag }} \ + bash -c "set -e && ctest --test-dir build/test" + + - name: Generate Coverage Report + run: | + docker run --rm --user "$(id -u):$(id -g)" -v ${{ github.workspace }}:/workspace ${{ inputs.image_tag }} \ + bash -c " \ + set -e \ + && cd build \ + && mkdir -p coverage \ + && gcovr -r .. \ + --exclude '.*/test/.*\.h' \ + --exclude '.*/test/.*\.cpp' \ + --decisions \ + --medium-threshold 50 --high-threshold 75 \ + --lcov coverage/filtered_coverage.info \ + --html-details coverage/index.html \ + --cobertura coverage.cobertura.xml \ + " + + - name: Upload L1 coverage artifacts + uses: actions/upload-artifact@v4 + with: + name: artifacts-L1-client + path: ${{ github.workspace }}/build/coverage/ + if-no-files-found: error + + - name: Code Coverage Summary Report + uses: irongut/CodeCoverageSummary@51cc3a756ddcd398d447c044c02cb6aa83fdae95 + with: + filename: build/coverage.cobertura.xml + badge: true + hide_complexity: true + format: markdown + output: both + thresholds: "50 75" + fail_below_min: false + + - name: Add coverage summary to job summary + run: cat code-coverage-results.md >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/L2-tests.yml b/.github/workflows/L2-tests.yml new file mode 100644 index 0000000..5bfb685 --- /dev/null +++ b/.github/workflows/L2-tests.yml @@ -0,0 +1,141 @@ +name: L2-tests + +on: + workflow_call: + inputs: + image_tag: + required: true + type: string + protocol: + required: true + type: string + +permissions: + contents: read + packages: read + +defaults: + run: + shell: bash + +env: + HUSKY: 0 + MOCK_PATH: ${{ github.workspace }}/mock-firebolt + MOCK_BRANCH: topic/changes-for-bidirectional + MOCK_SHA1SUM: 1fec7b75190e75ac8ea8ebf9f3e00c0a070b2566 + +jobs: + component_tests: + name: Run component tests + runs-on: ubuntu-latest + steps: + - name: Log in to the Container registry + uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Checkout Code + uses: actions/checkout@v4 + + - name: Download build directory + uses: actions/download-artifact@v4 + with: + name: build-dir + path: ${{ github.workspace }}/build + + - name: Setup Mock-Firebolt + id: setup-mock + uses: ./.github/actions/setup-mock + + - name: Restore Mock-Firebolt Cache + uses: actions/cache@v4 + with: + path: ${{ env.MOCK_PATH }} + key: ${{ runner.os }}-mock-${{ steps.setup-mock.outputs.deps_cache_key }} + + - name: Run Component Tests + run: | + chmod +x ${{ github.workspace }}/build/test/ctApp + docker run --rm --user "$(id -u):$(id -g)" -v ${{ github.workspace }}:/workspace -v ${{ env.MOCK_PATH }}:/mock ${{ inputs.image_tag }} \ + ./.github/scripts/run-component-tests.sh \ + --mock /mock \ + --protocol ${{ inputs.protocol }} \ + --config /workspace/.github/mock-firebolt/config.json \ + --openrpc /workspace/docs/openrpc/the-spec/firebolt-open-rpc.json \ + --app-openrpc /workspace/docs/openrpc/the-spec/firebolt-app-open-rpc.json \ + --test-exe /workspace/build/test/ctApp + + - name: Generate Component Coverage + run: | + docker run --rm --user "$(id -u):$(id -g)" -v ${{ github.workspace }}:/workspace ${{ inputs.image_tag }} \ + bash -c " \ + set -e \ + && cd build \ + && mkdir -p coverage \ + && gcovr -r .. \ + --exclude '.*/test/.*\.h' \ + --exclude '.*/test/.*\.cpp' \ + --lcov coverage/filtered_coverage.info \ + " + + - name: Upload L2 coverage artifact + uses: actions/upload-artifact@v4 + with: + name: artifacts-L2-client + path: ${{ github.workspace }}/build/coverage/filtered_coverage.info + if-no-files-found: error + + api_test_app: + name: Run API test app + runs-on: ubuntu-latest + steps: + - name: Log in to the Container registry + uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Checkout Code + uses: actions/checkout@v4 + + - name: Download build directory + uses: actions/download-artifact@v4 + with: + name: build-dir + path: ${{ github.workspace }}/build + + - name: Build api-test-app standalone + run: | + docker run --rm --user "$(id -u):$(id -g)" -v ${{ github.workspace }}:/workspace ${{ inputs.image_tag }} \ + bash -c " \ + set -e \ + && cd test/api_test_app \ + && cmake -B build -S . \ + -DCMAKE_BUILD_TYPE=Debug \ + -DCMAKE_PREFIX_PATH=/workspace/build/install \ + && cmake --build build --parallel \ + " + + - name: Setup Mock-Firebolt + id: setup-mock + uses: ./.github/actions/setup-mock + + - name: Restore Mock-Firebolt Cache + uses: actions/cache@v4 + with: + path: ${{ env.MOCK_PATH }} + key: ${{ runner.os }}-mock-${{ steps.setup-mock.outputs.deps_cache_key }} + + - name: Run api-test-app with mock server + run: | + chmod +x ${{ github.workspace }}/test/api_test_app/build/api-test-app + docker run --rm --user "$(id -u):$(id -g)" -v ${{ github.workspace }}:/workspace -v ${{ env.MOCK_PATH }}:/mock ${{ inputs.image_tag }} \ + ./.github/scripts/run-api-test-app.sh \ + --mock /mock \ + --config /workspace/.github/mock-firebolt/config.json \ + --openrpc /workspace/docs/openrpc/the-spec/firebolt-open-rpc.json \ + --app-openrpc /workspace/docs/openrpc/the-spec/firebolt-app-open-rpc.json \ + --test-exe /workspace/test/api_test_app/build/api-test-app \ No newline at end of file diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 17a176d..5cd1671 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -24,7 +24,7 @@ on: jobs: release: - if: github.ref_type == 'branch' && (github.ref_name == 'main' || endsWith(github.ref_name, '.rc') || endsWith(github.ref_name, '.x-maintenance')) + if: github.ref_type == 'branch' && (github.ref_name == 'main' || endsWith(github.ref_name, '.rc')) runs-on: ubuntu-latest concurrency: group: release @@ -46,11 +46,11 @@ jobs: run: | branch="${GITHUB_REF_NAME}" case "$branch" in - main|*.rc|*.*.x-maintenance) + main|*.rc) echo "Release allowed from branch: $branch" ;; *) - echo "Release blocked: branch '$branch' is not allowed. Use 'main', a '*.rc' branch, or a '*.*.x-maintenance' branch." + echo "Release blocked: branch '$branch' is not allowed. Use 'main' or a branch ending in '.rc'." exit 1 ;; esac @@ -135,10 +135,10 @@ jobs: run: | if [ "$IS_PROMOTION" = "true" ]; then if ! git merge-base --is-ancestor "${BASELINE}" HEAD; then - echo "ERROR: ${BASELINE} is not an ancestor of HEAD; select a descendant commit from this branch before promoting to stable." + echo "ERROR: ${BASELINE} is neither an ancestor of nor equal to HEAD; promote from a branch containing the RC commit." exit 1 fi - echo "Promoting ${BASELINE} to stable release; current HEAD is a descendant." + echo "Promoting ${BASELINE} to stable; the RC commit is HEAD or an ancestor of HEAD." exit 0 fi # BASELINE may not be a true git ancestor of HEAD (e.g. it was squash-merged @@ -170,7 +170,7 @@ jobs: - name: Create tag env: TAG: ${{ steps.version.outputs.next }} - GH_TOKEN: ${{ secrets.SEMANTIC_RELEASE_TOKEN }} + GH_TOKEN: ${{ secrets.AUTOMATION_TOKEN }} shell: bash run: | if git ls-remote --exit-code --tags origin "refs/tags/${TAG}" >/dev/null 2>&1; then @@ -182,7 +182,7 @@ jobs: echo "Tag ${TAG} already exists at the current HEAD; resuming the release." exit 0 fi - # Identity is just for the annotated tag's metadata -- SEMANTIC_RELEASE_TOKEN is + # Identity is just for the annotated tag's metadata -- AUTOMATION_TOKEN is # fine for this lookup, it doesn't touch ref creation. author_login="$(gh api user --jq '.login')" author_name="$(gh api user --jq '.name // .login')" @@ -206,7 +206,7 @@ jobs: - name: Create GitHub release env: - GH_TOKEN: ${{ secrets.SEMANTIC_RELEASE_TOKEN }} + GH_TOKEN: ${{ secrets.AUTOMATION_TOKEN }} TAG: ${{ steps.version.outputs.next }} IS_RC: ${{ steps.version.outputs.is_rc }} ARCHIVE: ${{ steps.asset.outputs.archive }} diff --git a/.github/workflows/sync-develop-to-main.yml b/.github/workflows/sync-develop-to-main.yml index 1b72a99..75c6447 100644 --- a/.github/workflows/sync-develop-to-main.yml +++ b/.github/workflows/sync-develop-to-main.yml @@ -2,7 +2,7 @@ # # The sync logic lives in app-gateway-automation/actions/sync-branches. # -# Optional secret: SEMANTIC_RELEASE_TOKEN (contents:write + pull-requests:write + issues:write) +# Optional secret: AUTOMATION_TOKEN (contents:write + pull-requests:write + issues:write) # Used for sync operations when present; falls back to github.token if absent. name: Sync develop to main @@ -39,4 +39,4 @@ jobs: with: source_branch: ${{ github.event.inputs.source_branch || 'develop' }} target_branch: ${{ github.event.inputs.target_branch || 'main' }} - token: ${{ secrets.SEMANTIC_RELEASE_TOKEN || github.token }} + token: ${{ secrets.AUTOMATION_TOKEN || github.token }} diff --git a/.github/workflows/ci.yml b/.github/workflows/tests-trigger.yml similarity index 56% rename from .github/workflows/ci.yml rename to .github/workflows/tests-trigger.yml index 81f814b..0f7a981 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/tests-trigger.yml @@ -1,4 +1,4 @@ -name: CI (c++) Build and Test +name: main-workflow on: workflow_dispatch: @@ -12,9 +12,9 @@ on: - rpc_v2 - legacy push: - branches: [ main, develop, next ] + branches: [ main, develop, '*.rc' ] pull_request: - branches: [ main, develop, next ] + branches: [ main, develop, '*.rc' ] defaults: run: @@ -22,9 +22,6 @@ defaults: env: HUSKY: 0 - MOCK_PATH: ${{ github.workspace }}/mock-firebolt - MOCK_BRANCH: topic/changes-for-bidirectional - MOCK_SHA1SUM: 1fec7b75190e75ac8ea8ebf9f3e00c0a070b2566 jobs: build_docker: @@ -36,7 +33,7 @@ jobs: image_tag: ${{ steps.meta.outputs.tags }} steps: - name: Log in to the Container registry - uses: docker/login-action@v3 + uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 with: registry: ghcr.io username: ${{ github.actor }} @@ -81,7 +78,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Log in to the Container registry - uses: docker/login-action@v3 + uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 with: registry: ghcr.io username: ${{ github.actor }} @@ -106,7 +103,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Log in to the Container registry - uses: docker/login-action@v3 + uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 with: registry: ghcr.io username: ${{ github.actor }} @@ -156,206 +153,28 @@ jobs: name: build-dir path: ${{ github.workspace }}/build - unit_tests: + trigger-L1: permissions: contents: read packages: read needs: [build_docker, build_project] - runs-on: ubuntu-latest - steps: - - name: Log in to the Container registry - uses: docker/login-action@v3 - with: - registry: ghcr.io - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Checkout Code - uses: actions/checkout@v4 - - - name: Download build directory - uses: actions/download-artifact@v4 - with: - name: build-dir - path: ${{ github.workspace }}/build - - - name: Run Unit Tests - run: | - chmod +x ${{ github.workspace }}/build/test/utApp - docker run --rm --user "$(id -u):$(id -g)" -v ${{ github.workspace }}:/workspace ${{ needs.build_docker.outputs.image_tag }} \ - bash -c " \ - set -e \ - && ctest --test-dir build/test \ - " - - - name: Generate Coverage Report - run: | - docker run --rm --user "$(id -u):$(id -g)" -v ${{ github.workspace }}:/workspace ${{ needs.build_docker.outputs.image_tag }} \ - bash -c " \ - set -e \ - && cd build \ - && mkdir -p coverage \ - && gcovr -r .. \ - --exclude '.*/test/.*\.h' \ - --exclude '.*/test/.*\.cpp' \ - --decisions \ - --medium-threshold 50 --high-threshold 75 \ - --lcov coverage/filtered_coverage.info \ - --html-details coverage/index.html \ - --cobertura coverage.cobertura.xml \ - " - - - name: Upload Coverage Report - uses: actions/upload-artifact@v4 - with: - name: coverage-report - path: ${{ github.workspace }}/build/coverage/ - - - name: Upload unit test lcov coverage - uses: actions/upload-artifact@v4 - with: - name: coverage-unit - path: ${{ github.workspace }}/build/coverage/filtered_coverage.info - - - name: Code Coverage Summary Report - uses: irongut/CodeCoverageSummary@v1.3.0 - with: - filename: build/coverage.cobertura.xml - badge: true - hide_complexity: true - format: markdown - output: both - thresholds: "50 75" - fail_below_min: false - - - name: Add coverage summary to job summary - run: cat code-coverage-results.md >> $GITHUB_STEP_SUMMARY - - component_tests: - permissions: - contents: read - packages: read - needs: [build_docker, build_project] - runs-on: ubuntu-latest - steps: - - name: Log in to the Container registry - uses: docker/login-action@v3 - with: - registry: ghcr.io - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Checkout Code - uses: actions/checkout@v4 - - - name: Download build directory - uses: actions/download-artifact@v4 - with: - name: build-dir - path: ${{ github.workspace }}/build - - - name: Setup Mock-Firebolt - id: setup-mock - uses: ./.github/actions/setup-mock - - - name: Restore Mock-Firebolt Cache - uses: actions/cache@v4 - with: - path: ${{ env.MOCK_PATH }} - key: ${{ runner.os }}-mock-${{ steps.setup-mock.outputs.deps_cache_key }} - - - name: Run Component Tests - run: | - # Temporary exclusion for TextToSpeech event tests. See docs/ci/text-to-speech-component-test-exclusion.md. - EXCLUDED_COMPONENT_TESTS="TextToSpeechCTest.subscribeOnWillSpeak:TextToSpeechCTest.subscribeOnSpeechStart:TextToSpeechCTest.subscribeOnSpeechComplete:TextToSpeechCTest.subscribeOnSpeechPause:TextToSpeechCTest.subscribeOnSpeechResume:TextToSpeechCTest.subscribeOnSpeechInterrupted:TextToSpeechCTest.subscribeOnNetworkError:TextToSpeechCTest.subscribeOnPlaybackError" - chmod +x ${{ github.workspace }}/build/test/ctApp - docker run --rm --user "$(id -u):$(id -g)" -e GTEST_FILTER="-$EXCLUDED_COMPONENT_TESTS" -v ${{ github.workspace }}:/workspace -v ${{ env.MOCK_PATH }}:/mock ${{ needs.build_docker.outputs.image_tag }} \ - ./.github/scripts/run-component-tests.sh \ - --mock /mock \ - --protocol ${{ github.event.inputs.protocol || 'rpc_v2' }} \ - --config /workspace/.github/mock-firebolt/config.json \ - --openrpc /workspace/docs/openrpc/the-spec/firebolt-open-rpc.json \ - --app-openrpc /workspace/docs/openrpc/the-spec/firebolt-app-open-rpc.json \ - --test-exe /workspace/build/test/ctApp - - - name: Generate Coverage Report - run: | - docker run --rm --user "$(id -u):$(id -g)" -v ${{ github.workspace }}:/workspace ${{ needs.build_docker.outputs.image_tag }} \ - bash -c " \ - set -e \ - && cd build \ - && mkdir -p coverage \ - && gcovr -r .. \ - --exclude '.*/test/.*\.h' \ - --exclude '.*/test/.*\.cpp' \ - --lcov coverage/filtered_coverage.info \ - " - - - name: Upload component test lcov coverage - uses: actions/upload-artifact@v4 - with: - name: coverage-component - path: ${{ github.workspace }}/build/coverage/filtered_coverage.info + uses: ./.github/workflows/L1-tests.yml + with: + image_tag: ${{ needs.build_docker.outputs.image_tag }} - api_test_app: + trigger-L2: permissions: contents: read packages: read needs: [build_docker, build_project] - runs-on: ubuntu-latest - steps: - - name: Log in to the Container registry - uses: docker/login-action@v3 - with: - registry: ghcr.io - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Checkout Code - uses: actions/checkout@v4 - - - name: Download build directory - uses: actions/download-artifact@v4 - with: - name: build-dir - path: ${{ github.workspace }}/build - - - name: Build api-test-app standalone - run: | - docker run --rm --user "$(id -u):$(id -g)" -v ${{ github.workspace }}:/workspace ${{ needs.build_docker.outputs.image_tag }} \ - bash -c " \ - set -e \ - && cd test/api_test_app \ - && cmake -B build -S . \ - -DCMAKE_BUILD_TYPE=Debug \ - -DCMAKE_PREFIX_PATH=/workspace/build/install \ - && cmake --build build --parallel \ - " - - - name: Setup Mock-Firebolt - id: setup-mock - uses: ./.github/actions/setup-mock - - - name: Restore Mock-Firebolt Cache - uses: actions/cache@v4 - with: - path: ${{ env.MOCK_PATH }} - key: ${{ runner.os }}-mock-${{ steps.setup-mock.outputs.deps_cache_key }} - - - name: Run api-test-app with mock server - run: | - chmod +x ${{ github.workspace }}/test/api_test_app/build/api-test-app - docker run --rm --user "$(id -u):$(id -g)" -v ${{ github.workspace }}:/workspace -v ${{ env.MOCK_PATH }}:/mock ${{ needs.build_docker.outputs.image_tag }} \ - ./.github/scripts/run-api-test-app.sh \ - --mock /mock \ - --config /workspace/.github/mock-firebolt/config.json \ - --openrpc /workspace/docs/openrpc/the-spec/firebolt-open-rpc.json \ - --app-openrpc /workspace/docs/openrpc/the-spec/firebolt-app-open-rpc.json \ - --test-exe /workspace/test/api_test_app/build/api-test-app + uses: ./.github/workflows/L2-tests.yml + with: + image_tag: ${{ needs.build_docker.outputs.image_tag }} + protocol: ${{ github.event.inputs.protocol || 'rpc_v2' }} coverage_gate: name: Coverage Gate - needs: [unit_tests, component_tests] + needs: [trigger-L1, trigger-L2] runs-on: ubuntu-latest permissions: contents: read @@ -393,14 +212,14 @@ jobs: continue-on-error: true uses: actions/download-artifact@v4 with: - name: coverage-unit + name: artifacts-L1-client path: ./unit-coverage - name: Download component test coverage artifact continue-on-error: true uses: actions/download-artifact@v4 with: - name: coverage-component + name: artifacts-L2-client path: ./component-coverage - name: Compare coverage to baseline @@ -418,9 +237,9 @@ jobs: if: >- github.event_name == 'push' && github.ref == 'refs/heads/develop' && - needs.unit_tests.result == 'success' && - needs.component_tests.result == 'success' - needs: [unit_tests, component_tests] + needs.trigger-L1.result == 'success' && + needs.trigger-L2.result == 'success' + needs: [trigger-L1, trigger-L2] runs-on: ubuntu-latest permissions: contents: write @@ -458,13 +277,13 @@ jobs: - name: Download unit test coverage artifact uses: actions/download-artifact@v4 with: - name: coverage-unit + name: artifacts-L1-client path: ./unit-coverage - name: Download component test coverage artifact uses: actions/download-artifact@v4 with: - name: coverage-component + name: artifacts-L2-client path: ./component-coverage - name: Extract coverage and write new baseline diff --git a/.transport.version b/.transport.version index 9ee1f78..ccad953 100644 --- a/.transport.version +++ b/.transport.version @@ -1 +1 @@ -1.1.11 +1.1.12 diff --git a/README.md b/README.md index eabed95..2a388b7 100644 --- a/README.md +++ b/README.md @@ -20,10 +20,6 @@ Examples: For the device websocket tunnel, use `setup-device-tunnel.sh`. Before running it, export `DEVICE_SSH_USER`, `DEVICE_SSH_HOST`, and `DEVICE_SSH_PORT`. -Temporary CI note: - -- `component_tests` currently excludes a small TextToSpeech event-test set while an OpenRPC/mock/test-fixture mismatch is resolved. See [docs/ci/text-to-speech-component-test-exclusion.md](docs/ci/text-to-speech-component-test-exclusion.md). - ## Lint Use `lint.sh` to run the same clang-format lint that CI enforces. diff --git a/docs/ci/text-to-speech-component-test-exclusion.md b/docs/ci/text-to-speech-component-test-exclusion.md deleted file mode 100644 index 1faf5b4..0000000 --- a/docs/ci/text-to-speech-component-test-exclusion.md +++ /dev/null @@ -1,45 +0,0 @@ -# Temporary Exclusion: TextToSpeech Component Event Tests - -## Status - -- Active temporary exclusion in CI `component_tests` job. -- Implemented via `GTEST_FILTER` in [.github/workflows/ci.yml](.github/workflows/ci.yml). - -## Excluded Tests - -- `TextToSpeechCTest.subscribeOnWillSpeak` -- `TextToSpeechCTest.subscribeOnSpeechStart` -- `TextToSpeechCTest.subscribeOnSpeechComplete` -- `TextToSpeechCTest.subscribeOnSpeechPause` -- `TextToSpeechCTest.subscribeOnSpeechResume` -- `TextToSpeechCTest.subscribeOnSpeechInterrupted` -- `TextToSpeechCTest.subscribeOnNetworkError` -- `TextToSpeechCTest.subscribeOnPlaybackError` - -## Root Cause (RC) - -- The TextToSpeech component tests inject event payload objects such as `{ "speechid": 1 }` in [test/component/textToSpeechTest.cpp](test/component/textToSpeechTest.cpp). -- In the current fixture OpenRPC consumed by Mock Firebolt, `TextToSpeech.on*` event method results are modeled as `null` in [docs/openrpc/the-spec/firebolt-open-rpc.json](docs/openrpc/the-spec/firebolt-open-rpc.json). -- During component test execution, Mock Firebolt validates injected event payloads against that `null` schema, rejects them, and no callback is delivered. -- Resulting failure signature is a timeout in [test/utils.cpp](test/utils.cpp) (`Did not receive event within timeout`). - -## Why Exclusion Is Temporary and Safe - -- This is a contract/fixture mismatch in event-shape validation, not a regression introduced by sync-workflow installation changes. -- The excluded set is narrowly scoped to the affected TextToSpeech event tests only. -- All other component tests continue to execute. - -## Resolution Plan - -Pick one of these and apply consistently across specs, mock, and tests: - -1. Spec-first fix: -- Update OpenRPC TextToSpeech event result schemas to the intended payload shape (for example including `speechid`), then regenerate/align fixtures and rerun component tests. - -2. Test-first fix: -- Keep OpenRPC event result schema as `null`, and update TextToSpeech component tests and event expectations so payload validation matches the current schema. - -## Exit Criteria (remove exclusion) - -- All 8 excluded TextToSpeech event tests pass in CI component tests without `GTEST_FILTER` exclusion. -- This document is updated or removed, and exclusion lines are deleted from [.github/workflows/ci.yml](.github/workflows/ci.yml). \ No newline at end of file