Skip to content

feat(cli): implement test command and reporter#1390

Open
johnyob wants to merge 1 commit intoajob410@jstz-test-executionfrom
ajob410@jstz-test-cli
Open

feat(cli): implement test command and reporter#1390
johnyob wants to merge 1 commit intoajob410@jstz-test-executionfrom
ajob410@jstz-test-cli

Conversation

@johnyob
Copy link
Collaborator

@johnyob johnyob commented Nov 19, 2025

Context

This PR implements the jstz test command and a pretty TTY reporter for test results

Description

Dependencies: #1389 #1378

When reviewing the PR, ignore test/reporter.rs, this is a selective copy of:

Within test/mod.rs:

Manually testing the PR

cargo test --package jstz_cli --lib --features v2_runtime -- test::tests
cargo test --package jstz_cli --test test --features v2_runtime

@johnyob johnyob force-pushed the ajob410@jstz-test-execution branch from 58ecac8 to 20a5497 Compare November 19, 2025 16:30
@johnyob johnyob force-pushed the ajob410@jstz-test-cli branch from 79899d2 to 6fb3fe4 Compare November 19, 2025 16:31
@johnyob johnyob force-pushed the ajob410@jstz-test-cli branch from 6fb3fe4 to b0e0444 Compare November 26, 2025 11:16
@johnyob johnyob requested a review from zcabter November 26, 2025 11:19
@johnyob johnyob self-assigned this Nov 26, 2025
@codecov
Copy link

codecov bot commented Nov 26, 2025

Codecov Report

❌ Patch coverage is 50.95137% with 464 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.12%. Comparing base (20a5497) to head (b0e0444).

Files with missing lines Patch % Lines
crates/jstz_cli/src/test/reporter.rs 38.74% 349 Missing and 21 partials ⚠️
crates/jstz_cli/src/test/mod.rs 72.43% 85 Missing and 9 partials ⚠️
Files with missing lines Coverage Δ
crates/jstz_cli/src/lib.rs 68.08% <100.00%> (+0.69%) ⬆️
crates/jstz_proto/src/runtime/v2/mod.rs 97.50% <ø> (ø)
crates/jstz_cli/src/test/mod.rs 72.43% <72.43%> (ø)
crates/jstz_cli/src/test/reporter.rs 38.74% <38.74%> (ø)

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20a5497...b0e0444. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

mod reporter;

#[derive(Debug, Clone, Args)]
pub struct TestArgs {
Copy link
Collaborator

@zcabter zcabter Dec 2, 2025

Choose a reason for hiding this comment

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

Jstz test as of today can only be used with ESM files right? There should be some documentation about this

deno_terminal = { workspace = true, optional = true }
indexmap = { workspace = true, optional = true }
jstz_runtime = { path = "../jstz_runtime", optional = true }
jstz_node = { path = "../jstz_node", optional = true }
Copy link
Collaborator

@zcabter zcabter Dec 2, 2025

Choose a reason for hiding this comment

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

I think this isn't used anywhere? Can be removed

TEST_RUNNER_MAX_SMART_FUNCTION_CALL_COUNT,
>::default()
.try_acquire()
.expect("Failred to acquire limiter for test runner"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.expect("Failred to acquire limiter for test runner"),
.expect("Failed to acquire limiter for test runner"),

@zcabter
Copy link
Collaborator

zcabter commented Dec 2, 2025

We need to update documentation as part of the test feature but that can be done in a separate PR

const TEST_RUNNER_ADDRESS: &str = "KT1RJ6PbjHpwc3M5rw5s2Nbmefwbuwbdxton";
const TEST_RUNNER_MAX_SMART_FUNCTION_CALL_COUNT: u8 = MAX_SMART_FUNCTION_CALL_COUNT;

pub(crate) async fn exec(args: TestArgs) -> anyhow::Result<()> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

For benefit of future maintainers, its probably a good idea to have either a README.md or doc string of the test framework architecture and an overview of how it works

@zcabter
Copy link
Collaborator

zcabter commented Dec 2, 2025

A few things that come to mind that need to be done as part of this feature but can be done as separate PRs

  1. Update docs
  2. Dockerfile needs to be updated to publish the v2 runtime cli

const TEST_RUNNER_MAX_SMART_FUNCTION_CALL_COUNT: u8 = MAX_SMART_FUNCTION_CALL_COUNT;

pub(crate) async fn exec(args: TestArgs) -> anyhow::Result<()> {
for file in args.files {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If files are empty, we should probably tell the user

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.

2 participants