Skip to content

ci: smoke test to verify PR test workflow#189

Closed
mcowger wants to merge 2 commits intomainfrom
ci/test-pr-v2
Closed

ci: smoke test to verify PR test workflow#189
mcowger wants to merge 2 commits intomainfrom
ci/test-pr-v2

Conversation

@mcowger
Copy link
Copy Markdown
Owner

@mcowger mcowger commented Apr 16, 2026

Trivial change to confirm the PR Tests workflow passes end-to-end after the workflow fix in #188 (build frontend before typecheck).

@github-actions
Copy link
Copy Markdown
Contributor

Review Summary

This PR adds a single comment line (// CI smoke test — verifies PR test workflow) to an existing test file. The change is minimal, harmless, and aligns with the stated goal of verifying the PR test workflow passes end-to-end after the fix in #188.

No issues found. No bugs, security concerns, performance problems, or style issues. The comment is clear and purposeful — it documents why the test file is being touched in a CI-fix PR.

Approved. 🟢

quota-enforcer and vision-fallthrough-e2e tests pass locally but fail
on CI due to in-memory SQLite and module caching divergence on Linux.
@github-actions
Copy link
Copy Markdown
Contributor

Review Summary

Bug: --exclude is not a valid bun test flag

The most significant issue in this PR is that bun test does not support an --exclude flag. I verified this locally:

  • Running bun test --help shows no --exclude option
  • Running bun test --exclude test/quota-enforcer.test.ts silently ignores the flag and still runs the excluded file (which then fails with the drizzle-orm import error)

This means the CI workflow change at .github/workflows/pr-tests.yml:33-35 will not actually exclude those flaky tests. The tests will still run and fail on CI, just as before. The --exclude flag is being silently ignored.

Suggested alternatives:

  1. Use bun test with positional arguments to only include desired test directories (e.g., bun test src/__tests__)
  2. Add describe.skip() or test.skip() inside the flaky test files, conditionally gated on process.env.CI
  3. Use --test-name-pattern to filter by test name

Test file comment

The // CI smoke test — verifies PR test workflow comment added to getProviderTypes.test.ts is harmless but a bit misleading — this test already existed and already passes on CI. The comment could cause confusion about the test's actual purpose.

Summary

The core intent (verifying the PR test workflow passes after the #188 fix) is fine, but the --exclude approach does not work with bun test. This needs to be fixed before the workflow will actually succeed with the flaky tests excluded.

@mcowger mcowger closed this Apr 16, 2026
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