Skip to content

Conversation

devin-ai-integration[bot]
Copy link
Contributor

Closes #3403

Description

This PR improves error message readability in the Cadence testing framework by pretty-printing error messages with actual newlines and tabs instead of escaped sequences.

Problem

When tests fail with error messages containing newlines (\n) or tabs (\t), the testing framework was displaying them as escaped sequences, making the output hard to read:

error: I.Test.Test.Error(message: \"error: line 1\\nerror: line 2\\n\\ttab indented line\")

Solution

Added a formatValueForTestOutput helper function that:

  1. Detects Test.Error values and extracts the raw message string without escaping
  2. Recursively formats ScriptResult and TransactionResult to apply pretty-printing to nested errors
  3. Unwraps optional values (SomeValue) since error fields are optional types

Now errors display with actual formatting:

error: I.Test.Test.Error(message: error: line 1
error: line 2
	tab indented line)

Key Changes

  • stdlib/test_contract.go: Added formatValueForTestOutput and formatValueForTestOutputHelper functions
  • Modified newTestTypeExpectFunction to use the new formatter
  • stdlib/test_test.go: Added test case to verify newlines and tabs are not escaped

Review Focus

  • Type identification: Uses strings.HasSuffix(typeID, ".Test.Error") to identify Error types - verify this is robust
  • Optional unwrapping: The SomeValue unwrapping is critical since error fields are optional - this was the key bug
  • Recursive formatting: Only handles ScriptResult/TransactionResult/Error specially - confirm this covers the necessary cases
  • Test coverage: Single test case covers the main scenario, existing tests verify no regressions

Link to Devin run: https://app.devin.ai/sessions/d951b3c6a2f94efa97ac0a2f088b1f11
Requested by: [email protected]

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

- Add formatValueForTestOutput helper function to format values with pretty-printed errors
- Handle Test.Error values by showing raw message content instead of escaped newlines/tabs
- Support recursive formatting for ScriptResult and TransactionResult
- Unwrap optional values (SomeValue) to access nested Error values
- Add test to verify newlines and tabs in error messages are not escaped

Fixes #3403
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link

Benchstat comparison

  • Base branch: onflow:master
  • Base commit: 3ec0c24
Results

old.txtnew.txt
time/opdelta
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
ContractFunctionInvocation-4383µs ± 0%404µs ± 0%~(p=1.000 n=1+1)
ExportType/composite_type-4302ns ± 0%264ns ± 0%~(p=1.000 n=1+1)
ExportType/simple_type-478.0ns ± 0%78.0ns ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ImperativeFib-421.3µs ± 0%20.0µs ± 0%~(p=1.000 n=1+1)
InterpretRecursionFib-42.05ms ± 0%2.06ms ± 0%~(p=1.000 n=1+1)
NewInterpreter/new_interpreter-41.03µs ± 0%1.03µs ± 0%~(p=1.000 n=1+1)
NewInterpreter/new_sub-interpreter-4322ns ± 0%348ns ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
RuntimeFungibleTokenTransferInterpreter-4567µs ± 0%592µs ± 0%~(p=1.000 n=1+1)
RuntimeFungibleTokenTransferVM-4636µs ± 0%625µs ± 0%~(p=1.000 n=1+1)
RuntimeResourceDictionaryValues-42.62ms ± 0%2.62ms ± 0%~(p=1.000 n=1+1)
RuntimeResourceTracking-412.0ms ± 0%11.8ms ± 0%~(p=1.000 n=1+1)
RuntimeScriptNoop-415.5µs ± 0%14.9µs ± 0%~(p=1.000 n=1+1)
RuntimeVMInvokeContractImperativeFib-428.3µs ± 0%28.4µs ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ValueIsSubtypeOfSemaType-466.8ns ± 0%66.9ns ± 0%~(p=1.000 n=1+1)
 
alloc/opdelta
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
ContractFunctionInvocation-4152kB ± 0%152kB ± 0%~(p=1.000 n=1+1)
ExportType/composite_type-4120B ± 0%120B ± 0%~(all equal)
ExportType/simple_type-40.00B 0.00B ~(all equal)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ImperativeFib-48.30kB ± 0%8.30kB ± 0%~(all equal)
InterpretRecursionFib-41.19MB ± 0%1.19MB ± 0%~(all equal)
NewInterpreter/new_interpreter-4976B ± 0%976B ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-4232B ± 0%232B ± 0%~(all equal)
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
RuntimeFungibleTokenTransferInterpreter-4155kB ± 0%156kB ± 0%~(p=1.000 n=1+1)
RuntimeFungibleTokenTransferVM-4174kB ± 0%173kB ± 0%~(p=1.000 n=1+1)
RuntimeResourceDictionaryValues-41.76MB ± 0%1.76MB ± 0%~(p=1.000 n=1+1)
RuntimeResourceTracking-49.26MB ± 0%9.25MB ± 0%~(p=1.000 n=1+1)
RuntimeScriptNoop-47.99kB ± 0%7.99kB ± 0%~(p=1.000 n=1+1)
RuntimeVMInvokeContractImperativeFib-410.3kB ± 0%10.3kB ± 0%~(all equal)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ValueIsSubtypeOfSemaType-448.0B ± 0%48.0B ± 0%~(all equal)
 
allocs/opdelta
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
ContractFunctionInvocation-42.47k ± 0%2.47k ± 0%~(all equal)
ExportType/composite_type-43.00 ± 0%3.00 ± 0%~(all equal)
ExportType/simple_type-40.00 0.00 ~(all equal)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ImperativeFib-4176 ± 0%176 ± 0%~(all equal)
InterpretRecursionFib-417.7k ± 0%17.7k ± 0%~(all equal)
NewInterpreter/new_interpreter-415.0 ± 0%15.0 ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-44.00 ± 0%4.00 ± 0%~(all equal)
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
RuntimeFungibleTokenTransferInterpreter-42.93k ± 0%2.93k ± 0%~(all equal)
RuntimeFungibleTokenTransferVM-43.06k ± 0%3.06k ± 0%~(all equal)
RuntimeResourceDictionaryValues-436.7k ± 0%36.7k ± 0%~(all equal)
RuntimeResourceTracking-4159k ± 0%159k ± 0%~(p=1.000 n=1+1)
RuntimeScriptNoop-4113 ± 0%113 ± 0%~(all equal)
RuntimeVMInvokeContractImperativeFib-4213 ± 0%213 ± 0%~(all equal)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ValueIsSubtypeOfSemaType-41.00 ± 0%1.00 ± 0%~(all equal)
 

@turbolent
Copy link
Member

(aside) I'm not sure the solution is what we're looking for, will pick this up later when we have time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Cadence Testing Framework] Pretty Print Error Messages when running tests

1 participant