experiment: code coverage by johnstonmatt · Pull Request #676 · supabase/edge-runtime

johnstonmatt · 2026-03-24T03:21:12Z

What kind of change does this PR introduce?

Bug fix, feature, docs update, ...

What is the current behavior?

Please link any relevant issues here.

What is the new behavior?

Feel free to include screenshots if it includes visual changes.

Additional context

Add any other context or screenshots.

Add comprehensive unit tests to crates that previously had zero test coverage, targeting code most likely to break during the Deno v2.1.4 to v2.7.7 upgrade. - ext_runtime: rate limiter (12), metrics/promises (8), cert store (8) - ext_workers: permissions conversion (6), worker context/lifecycle (10) - base_rt: RuntimeState flag semantics (5) - base_mem_check: heap statistics serialization (5) - http_utils: upgrade detection, status emission (7)

Cover the core module loading infrastructure that interfaces with Deno's module system and eszip format: - metadata.rs: workspace resolver, npmrc, node_modules deserialization, static asset lookup, error paths (14 tests) - module_loader/util.rs: unsafe arc_u8_to_arc_str UTF-8 validation (5 tests) - lib.rs: strip_file_scheme, ensure_unix_relative_path utilities (6 tests) - eszip/error.rs: EszipError display formatting (2 tests - note: 1 test is counted in both error files)

Pure Rust unit tests for critical runtime infrastructure: - utils/units.rs: mib_to_bytes, bytes_to_display, human_elapsed, percentage_value (13 tests) - utils/json.rs: merge_object with nested/overlapping/null cases (7 tests) - worker/termination_token.rs: cancel semantics, child tokens, cancel_and_wait async behavior (8 tests) Note: base crate requires openblas for ONNX - tests verified via CI.

The ext_event_worker crate cannot compile for --lib tests in isolation because the uuid workspace dependency lacks the serde feature. Tests for this module should live in the integration test suite.

Copilot

Pull request overview

This PR primarily adds unit tests across multiple Rust crates/extensions to improve code coverage, plus a small documentation note explaining why ext_event_worker tests live in the integration test suite.

Changes:

Added #[cfg(test)] unit test modules across ext/* and crates/* for defaults, serialization, display formatting, and rate-limiting behavior.
Added coverage-oriented tests for small utility/helpers (JSON merge, units formatting, UTF-8 Arc conversion, header upgrade detection, etc.).
Added a module-level note in ext/event_worker/events.rs about test placement constraints.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
ext/workers/lib.rs	Adds unit tests for permissions options and worker create options deserialization/defaults.
ext/workers/context.rs	Adds unit tests for worker kinds/runtime opts, defaults, exit status, and timing status.
ext/runtime/rate_limit.rs	Adds unit tests for rate limit table + limiter behavior; also adds/duplicates a doc comment.
ext/runtime/lib.rs	Adds unit tests for shared metrics, promise metrics, shared stats, and rate limiter opts default.
ext/runtime/errors_rt.rs	Adds unit tests for error-class mapping helpers.
ext/runtime/cert.rs	Adds unit tests for certificate store selection/loading and error display.
ext/event_worker/events.rs	Adds note explaining why unit tests for this module are in integration tests.
crates/http_utils/src/utils.rs	Adds unit tests for upgrade header detection and `emit_status_code`.
crates/deno_facade/module_loader/util.rs	Adds unit tests for UTF-8 validation + Arc transmute helper.
crates/deno_facade/metadata.rs	Adds unit tests for metadata defaults and helper methods behavior.
crates/deno_facade/lib.rs	Adds unit tests for `strip_file_scheme` / `ensure_unix_relative_path`.
crates/deno_facade/eszip/error.rs	Adds unit tests for `EszipError` display formatting.
crates/base_rt/src/runtime_state.rs	Adds unit tests for runtime state flags defaulting and behavior.
crates/base_mem_check/src/lib.rs	Adds unit tests for heap statistics defaults and serialization.
crates/base/src/worker/termination_token.rs	Adds unit tests for termination token behavior (but currently has a compile issue).
crates/base/src/utils/units.rs	Adds unit tests for MiB conversion, display formatting, elapsed formatting, percentage computation.
crates/base/src/utils/json.rs	Adds unit tests for deep/overlapping JSON object merge behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T03:24:38Z

ext/runtime/rate_limit.rs

  /// Returns `Ok(())` when the request is allowed, or `Err(retry_after_ms)`
  /// with the number of milliseconds until the window resets when denied.


The doc comment for check_and_increment is duplicated (the same two lines appear twice), which is noisy and can confuse generated docs. Please remove the repeated lines so the method has a single, accurate doc comment.

Suggested change

/// Returns `Ok(())` when the request is allowed, or `Err(retry_after_ms)`

/// with the number of milliseconds until the window resets when denied.

Copilot · 2026-03-24T03:24:38Z

ext/runtime/rate_limit.rs

+    let ttl = Duration::from_millis(1);
+
+    // Exhaust the budget
+    table.check_and_increment("key1", 1, ttl).unwrap();
+    assert!(table.check_and_increment("key1", 1, ttl).is_err());
+
+    // Wait for TTL to expire
+    std::thread::sleep(Duration::from_millis(5));


table_resets_after_ttl_expires relies on very short real-time sleeps (1ms TTL, then sleeping 5ms). This is prone to flakiness on slow/contended CI runners. Consider using a larger TTL/sleep margin, or convert this to a Tokio test using tokio::time::pause/advance (or otherwise avoid wall-clock sleeps).

Suggested change

let ttl = Duration::from_millis(1);

// Exhaust the budget

table.check_and_increment("key1", 1, ttl).unwrap();

assert!(table.check_and_increment("key1", 1, ttl).is_err());

// Wait for TTL to expire

std::thread::sleep(Duration::from_millis(5));

let ttl = Duration::from_millis(50);

// Exhaust the budget

table.check_and_increment("key1", 1, ttl).unwrap();

assert!(table.check_and_increment("key1", 1, ttl).is_err());

// Wait for TTL to expire

std::thread::sleep(Duration::from_millis(200));

Copilot · 2026-03-24T03:24:38Z

crates/base/src/worker/termination_token.rs

+    // Spawn a task that cancels outbound after a short delay
+    tokio::spawn(async move {
+      tokio::time::sleep(Duration::from_millis(10)).await;
+      outbound.cancel();
+    });
+
+    let result =
+      tokio::time::timeout(Duration::from_secs(1), token.cancel_and_wait())
+        .await;
+


The new tests use Duration::from_millis/... but Duration is not imported anywhere in this module, so this won't compile. Add use std::time::Duration; in the test module (or qualify as std::time::Duration).

Copilot · 2026-03-24T03:24:38Z

ext/runtime/cert.rs

+  #[test]
+  fn get_root_cert_store_invalid_ca_data_bytes_errors() {
+    // Invalid PEM data should produce an error
+    let bad_pem = b"not a valid certificate\n";
+
+    let store = get_root_cert_store(
+      None,
+      Some(vec!["mozilla".into()]),
+      Some(CaData::Bytes(bad_pem.to_vec())),
+    );
+    // Invalid PEM is silently ignored (no certs parsed), so it succeeds
+    // but doesn't add any extra certs
+    assert!(store.is_ok());
+  }


This test is named get_root_cert_store_invalid_ca_data_bytes_errors and the comment says invalid PEM “should produce an error”, but the assertions expect Ok. Please align the test name/comment with the expected behavior (either assert an error, or rename/reword to reflect that invalid CA data is ignored).

Copilot · 2026-03-24T03:24:39Z

ext/runtime/cert.rs

+    // Mozilla trust store has many certs
+    assert!(store.len() > 100);


get_root_cert_store_mozilla asserts the Mozilla root store has len() > 100, which is a brittle threshold and could fail if the upstream root list changes. Consider asserting a weaker invariant (e.g., len() > 0) or checking that it contains at least one known root, to keep the test stable over time.

Suggested change

// Mozilla trust store has many certs

assert!(store.len() > 100);

// Mozilla trust store should not be empty

assert!(store.len() > 0);

johnstonmatt added 4 commits March 24, 2026 00:00

chore: add note about event_worker test limitations

685cf52

The ext_event_worker crate cannot compile for --lib tests in isolation because the uuid workspace dependency lacks the serde feature. Tests for this module should live in the integration test suite.

Copilot AI review requested due to automatic review settings March 24, 2026 03:21

Copilot started reviewing on behalf of johnstonmatt March 24, 2026 03:21 View session

style: apply rustfmt formatting to test files

8d886ec

Copilot AI reviewed Mar 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

experiment: code coverage#676

experiment: code coverage#676
johnstonmatt wants to merge 5 commits intomainfrom
FUNC/code-coverage

johnstonmatt commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		/// Returns `Ok(())` when the request is allowed, or `Err(retry_after_ms)`
		/// with the number of milliseconds until the window resets when denied.

		// Mozilla trust store has many certs
		assert!(store.len() > 100);

Uh oh!

Conversation

johnstonmatt commented Mar 24, 2026

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior?

Additional context

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants