minor improvements for deterministic tests in CI

[rsoury]

Summary by CodeRabbit

• Chores

  • Pinned Bun to 1.3.9 in CI for consistent builds.
  • Added a CI step to generate protobuf types before tests.

• Tests

  • Restored normal error logging during tests to improve visibility.
  • Converted several lifecycle hooks and tests to async and made teardowns await cleanup.
  • Standardized file access in tests to improve portability.

[coderabbitai]

Caution

Review failed

The pull request is closed.

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0573a96 and f7397d1.

📒 Files selected for processing (2)

• .github/workflows/ci.yml
• .github/workflows/publish.yml

---

Walkthrough

Pins Bun to 1.3.9 in CI workflows, adds a "Generate protobuf types" step, removes console.error mocking from test setup, converts several tests and lifecycle hooks to async/await, and replaces Bun-specific fs/path usage with Node ESM reads for policy.json.

Changes

Cohort / File(s)	Summary
CI Workflows .github/workflows/ci.yml, .github/workflows/publish.yml	Pin bun-version from latest to 1.3.9 and add a new step "Generate protobuf types" running bun run proto-gen after dependencies install.
Test setup test-setup.ts	Remove mocking/restoration of console.error; tests no longer silence error logs.
Async test lifecycle test/cex-broker.test.ts, test/otel.test.ts	Convert lifecycle hooks and tests to async and replace synchronous teardown calls with await broker.stop() / await metrics.close().
ESM file access in tests test/integration.test.ts	Replace require("bun:fs") and Bun path usage with import { readFileSync } from "node:fs" and new URL(..., import.meta.url) for reading policy.json.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Feat/added Otel #24 — related changes to Otel test lifecycle and async teardown (metrics.close()).

Suggested reviewers

• victorshevtsov

Poem

🐰 Bun pinned snug, the workflow wakes,

Proto types sprout from careful stakes,
No more muted errors in nightly tests,
Async hops tidy up the rests,
A carrot cheer for passing stakes! 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'minor improvements for deterministic tests in CI' directly relates to the main changes: pinning Bun version to 1.3.9 in CI workflows, converting tests to async for proper cleanup, and removing console.error mocking to ensure deterministic test behavior.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch develop

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

test/integration.test.ts (1)

7-18: Correct ESM path resolution pattern.

Using new URL("../policy/policy.json", import.meta.url) is the standard approach for resolving relative paths in ES modules.

Consider extracting the path constant to reduce duplication:

♻️ Optional: Extract shared path constant

 describe("Integration Tests", () => {
+	const policyPath = new URL("../policy/policy.json", import.meta.url);
+
 	describe("Policy Integration", () => {
 		test("should load and validate policy correctly", () => {
-			const policyPath = new URL("../policy/policy.json", import.meta.url);
-
 			expect(() => {
 				const policyData = readFileSync(policyPath, "utf8");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/integration.test.ts` around lines 7 - 18, The tests duplicate the ESM
path resolution for the policy file using new URL("../policy/policy.json",
import.meta.url); extract that into a single shared constant (e.g., const
policyPath = new URL("../policy/policy.json", import.meta.url)) at the top of
the test file and replace the inline occurrences inside the tests ("should load
policy file" and "should have correct policy structure") with the shared
policyPath variable so both readFileSync and JSON.parse use the same resolved
URL.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3533fdf and 1e1968c.

📒 Files selected for processing (6)

• .github/workflows/ci.yml
• .github/workflows/publish.yml
• test-setup.ts
• test/cex-broker.test.ts
• test/integration.test.ts
• test/otel.test.ts

💤 Files with no reviewable changes (1)

• test-setup.ts

🧰 Additional context used

🧬 Code graph analysis (1)

test/cex-broker.test.ts (1)

src/index.ts (1)

• CEXBroker (25-270)

🔇 Additional comments (7)

.github/workflows/ci.yml (1)

22-22: Good practice: Pinning the Bun version for CI reproducibility.

Using ^1.3.9 ensures consistent behavior across CI runs while still allowing patch updates for bug fixes. This aligns with the PR objective of deterministic tests.

test/cex-broker.test.ts (2)

45-49: Correct fix for test isolation.

The broker.stop() method is async (performs forceShutdown() and close() calls). Awaiting it in afterEach ensures resources are fully released before the next test runs, eliminating potential race conditions and flaky tests in CI.

---

212-216: Async handling is correct.

Awaiting the async stop() method ensures the test properly waits for shutdown completion before assertions.

.github/workflows/publish.yml (1)

25-25: Consistent version pinning across workflows.

Aligning the Bun version with ci.yml ensures the publish workflow tests run under the same conditions as CI tests.

test/otel.test.ts (2)

30-57: Proper async teardown for enabled OtelMetrics instances.

Adding await metrics.close() in tests that enable OTLP ensures the SDK's internal resources (exporters, readers) are properly shut down. This prevents resource leaks and cross-test interference that could cause flaky CI results.

---

61-96: Consistent cleanup pattern for env-based metrics.

Each test that creates an enabled OtelMetrics instance via environment variables now properly closes it, ensuring no lingering SDK state affects subsequent tests.

test/integration.test.ts (1)

1-1: Good migration to standard Node.js APIs.

Using node:fs instead of Bun-specific file system APIs improves test portability and aligns with standard ES module patterns.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@test/integration.test.ts`:
- Around line 7-18: The tests duplicate the ESM path resolution for the policy
file using new URL("../policy/policy.json", import.meta.url); extract that into
a single shared constant (e.g., const policyPath = new
URL("../policy/policy.json", import.meta.url)) at the top of the test file and
replace the inline occurrences inside the tests ("should load policy file" and
"should have correct policy structure") with the shared policyPath variable so
both readFileSync and JSON.parse use the same resolved URL.