onflow / cadence-tools

Developer tools for Cadence
https://www.onflow.org/
Apache License 2.0
24 stars 20 forks source link

[Test] Logging some fields crashes VS Code test runner #265

Closed austinkline closed 8 months ago

austinkline commented 8 months ago

Current Behavior

Logging a UFix64 type is crashing my vscode test runner, but doesn't crash my cli

./run-tests.sh
9:12AM INF LOG: 1.00000000

Test results: "tests/Flowty_tests.cdc"
- PASS: testPrintUFix
pub fun testPrintUFix() {
    log(1.0)
}

But if I run the same test on the vscode extension, it will crash:

image

This seems to happen consistently with any type I log, String and UInt64 both exhibit the same behavior

Expected Behavior

using log should not crash my tests when running directly in vs code

Steps To Reproduce

  1. Define the test:
    pub fun testPrintUFix() {
        log(1.0)
    }
  2. Run the test in vscode, there should be a play button on the left side of your test:

https://github.com/onflow/cadence-tools/assets/8711160/1fabf8bc-15c7-4f2c-abb9-3679100f02d1

Environment

- Cadence version: v1.9.2
- Network: Testing Framework
jribbink commented 8 months ago

Ahh nice catch... That's a bit of a nasty one - has to do with CLI output not being valid JSON despite the --output=json flag when printing the logs. The fix here is to either suppress the logs or capture them and format them properly into the output.

Do you see the logs as useful to surface within the VSCode Test Runner (i.e. in the test output within VSCode) @austinkline? Or would suppressing them so that the runner works be a satisfactory solution?

turbolent commented 8 months ago

@jribbink So is this a bug in the test framework, CLI, LS, or in the VS Code extension?

austinkline commented 8 months ago

Do you see the logs as useful to surface within the VSCode Test Runner (i.e. in the test output within VSCode) @austinkline? Or would suppressing them so that the runner works be a satisfactory solution?

So I didn't even realize the test runner was an option until I saw the Play button next to it. Before seeing that option, I used to comment out all the tests I didn't want run, and would use the cli since I thought that was my only option.

In an ideal world, logs would show in the vscode output if that's the way folks are expected to write new tests. At in my opinion, that's a much more friendly journey to using the testing framework. Logs are probably an important part of that because there aren't any ways to add debug breakpoints currently, so logs are your only other main avenue besides something like adding the following to your tests

assert(false, message: "here!")

That being said, having weird failures that crash a test is far worse than just suppressing logs. Not the end of the world to start with preventing the issue first, then work towards getting them to show up later

jribbink commented 8 months ago

@turbolent It's a CLI issue that probably needs lower-level support from the test framework to be able to capture (or suppress) logs instead of printing to stdio

jribbink commented 8 months ago

@turbolent @austinkline https://github.com/onflow/cadence-tools/pull/267 should fix this once integrated with Flow CLI