Skip to content

Conversation

@AlexD10S
Copy link
Collaborator

This PR improves the error message when building specs fails, making it clearer and more aligned with the output of the underlying command.

Before:

Running pop build spec --chain ./wrong-chain-spec:

Error: IO error: command ["/templates_test/evm/target/release/parachain-template-node", "build-spec", "--chain", "./wrong-chain-spec", "--disable-default-bootnode"] exited with code 1

Caused by:
    command ["templates_test/evm/target/release/parachain-template-node", "build-spec", "--chain", "./wrong-chain-spec", "--disable-default-bootnode"] exited with code 1

This error message was not very informative and did not clearly indicate the root cause of the failure.
Now:

 Failed to build the chain spec. Error: Input("Error opening spec file `./wrong-chain-spec`: No such file or directory (os error 2)")

This message explicitly states the failure reason and matches the output of the underlying command:

./target/release/parachain-template-node build-spec --chain ./wrong-chain-spec
Error: Input("Error opening spec file `./c`: No such file or directory (os error 2)")

Closes #475

[sc-3371]

@codecov
Copy link

codecov bot commented Mar 20, 2025

Codecov Report

Attention: Patch coverage is 83.07692% with 11 lines in your changes missing coverage. Please review.

Project coverage is 75.18%. Comparing base (1308c69) to head (d42cb81).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
crates/pop-parachains/src/build/mod.rs 78.37% 1 Missing and 7 partials ⚠️
crates/pop-cli/src/commands/build/spec.rs 0.00% 3 Missing ⚠️
@@            Coverage Diff             @@
##             main     #477      +/-   ##
==========================================
+ Coverage   75.11%   75.18%   +0.06%     
==========================================
  Files          70       71       +1     
  Lines       14967    15053      +86     
  Branches    14967    15053      +86     
==========================================
+ Hits        11243    11318      +75     
- Misses       2253     2255       +2     
- Partials     1471     1480       +9     
Files with missing lines Coverage Δ
crates/pop-parachains/src/errors.rs 100.00% <100.00%> (ø)
crates/pop-cli/src/commands/build/spec.rs 72.30% <0.00%> (+0.39%) ⬆️
crates/pop-parachains/src/build/mod.rs 88.04% <78.37%> (-0.41%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@chungquantin chungquantin left a comment

Choose a reason for hiding this comment

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

Seems this PR's issue is already handled in the benchmarking. As benchmarking is not merged to main yet, my suggestion is to have a common method that can be used for both features and then I'll refactor the benchmarking feature after it is merged to main.

@AlexD10S AlexD10S requested a review from chungquantin March 24, 2025 08:34
Copy link
Contributor

@chungquantin chungquantin left a comment

Choose a reason for hiding this comment

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

Looks great!

@AlexD10S AlexD10S merged commit 68a68c6 into main Mar 24, 2025
20 checks passed
@AlexD10S AlexD10S deleted the fix/build-spec-error-messaging branch March 24, 2025 11:00
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.

Improve error messaging when building specs fails

3 participants