Skip to content

Enhance ACCESS_CARTA assertions in ICD tests - #91

Draft
markccchiang wants to merge 6 commits into
devfrom
mark/update_access_carta_tests
Draft

Enhance ACCESS_CARTA assertions in ICD tests#91
markccchiang wants to merge 6 commits into
devfrom
mark/update_access_carta_tests

Conversation

@markccchiang

@markccchiang markccchiang commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR addresses part of issue #3. The five ACCESS_CARTA tests cover the REGISTER_VIEWER handshake, but most of their assertions could not fail. protobufjs decodes an unset fixed32 as 0, an unset string as '' and an unset map as {}, so toBeDefined() passes on every field the backend never sets, and toMatchObject({}) matches any object. The tests were also absent from every stage file, so none of them ran in CI.

This branch replaces the vacuous checks with assertions grounded in the backend source, and adds the five tests to the file_browser stage.

Keys to all five tests

  • session_id is asserted against what the branch in Session::OnRegisterViewer actually does, not 0 for a new session, the requested id for a resumed one.
  • message must be non-empty and must report the session id. The backend emits a different sentence per branch, so the assertion matches on the id rather than the wording.
  • server_feature_flags is checked for the READ_ONLY bit rather than compared to 0, so running the backend with scripting enabled does not fail the suite.
  • platform_strings must carry non-empty release_info, deployment, architecture and platform entries, and platform must be macOS or Linux.
  • user_preferences and user_layouts use toEqual({}) instead of toMatchObject({}).

Per-test summary

ACCESS_CARTA_DEFAULT also pins session_type to NEW, and passes the connectTimeout that was declared but never used.

ACCESS_CARTA_DEFAULT_CONCURRENT was not concurrent: it awaited each connect() inside a for loop, so the 10 clients registered one after another. Replaced with Promise.all. Session ids are generated from a microsecond timestamp with no uniqueness check, so serial registration made the uniqueness assertion trivially true; registering in parallel now spaces the ids 11-16 microseconds apart and exercises the collision window the assertion is meant to guard. Also asserts that all 10 acknowledgements report the same server information.

ACCESS_CARTA_KNOWN_SESSION replaces a console.warn guarded by an always-true condition with a real assertion on the message.

ACCESS_CARTA_NO_CLIENT_FEATURE now matches the default test check for check, which is the point of the test: the backend never reads client_feature_flags, so no part of the response may depend on it. Also renames a test that claimed a numeric field was a non-empty string.

ACCESS_CARTA_SAME_ID_TWICE discarded the acknowledgement returned by connect() and asserted only the second one. Both are now captured, and their messages are asserted to report the id and to differ from each other, which is the only coverage of the reconnect branch.

Checklist

For the pull request:

  • Documentation has been updated (or no documentation changes are needed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant