Skip to content

fix: Web core test and CI workflow fixes#1000

Closed
jacobsimionato wants to merge 5 commits intogoogle:mainfrom
jacobsimionato:exit-gate-3
Closed

fix: Web core test and CI workflow fixes#1000
jacobsimionato wants to merge 5 commits intogoogle:mainfrom
jacobsimionato:exit-gate-3

Conversation

@jacobsimionato
Copy link
Copy Markdown
Collaborator

Description

This PR addresses multiple issues preventing the web_core tests from running successfully locally and in CI.

Description of Changes

  • GitHub Actions Workflow Fix: Added the missing runs-on: ubuntu-latest to the lint job in .github/workflows/web_build_and_test.yml and standardized the paths glob pattern.
  • Test Command Fix: Updated the test script in renderers/web_core/package.json to use a glob pattern (node --test "dist/**/*.test.js") instead of pointing to the directory, resolving MODULE_NOT_FOUND errors locally and in CI.
  • Linting Fix: Resolved @typescript-eslint/no-unused-vars linting errors in renderers/web_core/src/v0_9/catalog/types.test.ts by asserting the unused type-level variables at runtime.

Rationale

  • The missing runs-on property in the lint job caused GitHub Actions to mark the entire web_build_and_test.yml workflow as invalid, preventing both tests and linting from running.
  • Node's native test runner (node --test) can fail with module resolution errors when pointed directly at a directory (dist) in a "type": "module" project. Using a specific glob pattern bypasses this issue.
  • Type-only variables in test files triggered the linter, causing the npm run lint step to fail.

Testing/Running Instructions

  1. Pull down this branch locally.
  2. Navigate to the renderers/web_core directory: cd renderers/web_core
  3. Run tests: npm run test (should pass successfully).
  4. Run lint: npm run lint (should pass with no errors).
  5. Review the Actions tab in GitHub to ensure the Web core workflow now successfully triggers and completes.

Pre-launch Checklist

  • My code changes (if any) have tests.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates @a2ui/web_core to version 0.9.0, refines the test execution command to target specific test files, and replaces type-only variable declarations in tests with runtime assertions to satisfy linting requirements. The review feedback suggests using the void operator instead of assert.strictEqual for variables intended only for compile-time type validation, as this avoids unnecessary runtime checks while still silencing unused variable warnings.

// This happens when `mockApi: ComponentApi`, but doesn't when
// `mockApi {} satisfies ComponentApi`!
const inferredIsAny: IsAny<InferredType> = false;
assert.strictEqual(inferredIsAny, false);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

While using assert.strictEqual works to silence the 'no-unused-vars' lint error, it adds a runtime assertion for what is a compile-time type check. The purpose of inferredIsAny is to fail at compile time if the type is incorrect, not to be asserted at runtime.

A more idiomatic and clearer way to mark a variable as intentionally used for its type-level effects is to use the void operator. This signals that you are using the variable for its compile-time check and intentionally discarding its value, without adding a runtime assertion.

Suggested change
assert.strictEqual(inferredIsAny, false);
void inferredIsAny;

// typesMatchExact only accepts "true" if `TypesAreEquivalent`
const typesMatchExact: TypesAreEquivalent<ExpectedType, InferredType> =
true;
assert.strictEqual(typesMatchExact, true);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Similar to the other type-level check, using assert.strictEqual here adds a runtime assertion for a compile-time check.

To make the intent clearer and avoid a redundant runtime check, you can use the void operator to mark typesMatchExact as intentionally used for its type-level effects.

Suggested change
assert.strictEqual(typesMatchExact, true);
void typesMatchExact;

@github-project-automation github-project-automation bot moved this from Todo to Done in A2UI Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant