Skip to content

Refactor entrypoint so tests can import real functions instead of re-implementing them #297

Description

@felickz

Background

src/main.ts ends with a top-level side effect:

void run();

Because the module is ESM ("type": "module" in package.json) and this call executes immediately on import, every test file in __tests__/ avoids importing from src/main.ts directly — doing so would trigger the action to actually run (hitting core.getInput, GitHub API clients, etc.) as an unwanted side effect of the import itself.

Instead, each test file re-implements the function(s) under test locally (see alert-identifier.test.ts, error-handling.test.ts, get_rules_from_run.test.ts, and the newer get_tool_names.test.ts, alert_identifier_from_api_alert.test.ts, match_alerts.test.ts, fetch_alerts_by_identifier.test.ts added in #296). This is a pre-existing convention, not something introduced by #296.

The problem

This was flagged during review of #296 (see review comments, here, here, and here):

These tests re-implement X instead of importing the real implementation, so they can pass even if production code diverges from what's tested.

This is a real risk — it almost bit us in #296 itself: when fetch_alerts_by_identifier()'s implementation changed (to fetch both open and dismissed states explicitly), the test file's local re-implementation had to be manually kept in sync by hand. If that sync had been missed, the tests would have kept passing against stale logic while production code silently diverged.

Proposed fix

Guard the top-level invocation so importing src/main.ts for its exported functions doesn't trigger execution, e.g. (ESM-compatible entrypoint check):

// Only run when this module is the actual entrypoint (e.g. `node dist/index.js`),
// not when imported by tests.
if (import.meta.url === `file://${process.argv[1]}`) {
  void run();
}

This needs to be verified against how @vercel/ncc bundles the action for distribution (dist/index.js) to make sure the guard still resolves correctly bundled/at runtime as a GitHub Action.

Once that's in place, all __tests__/*.test.ts files can be updated to import { ... } from "../src/main.ts" instead of maintaining hand-rolled duplicates of the functions under test — eliminating the drift risk entirely.

Scope

This is a test-infrastructure-only change; no behavior change to the action itself. Kept out of #296 to keep that bug-fix PR focused and easy to review.

Activity

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

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions