feat(cmd): log test errors

shellcromancer commented 2 weeks ago

Description

This changes the CLIs "test" subcommand (substation test) to log runtime transform errors to standard error. This

Motivation and Context

When performing test file driven configuration development it's not clear if a specific tests [config error] is from misconfigured Jsonnet declarations, invalid Substation options or runtime errors. You can remove the first two options by diagnosing with substation vet but then you still don't know what the runtime transform error is if that's the case.

How Has This Been Tested?

Verified that multiple tests across files can be run, and are more clear when there was an opaque [config error].

$ substation test .
ok  fizz.libsonnet  607µs
?   buzz.libsonnet  [transform error]
error: transform 907268ad-3907ac72: format 2006-01-02T15:04:05.000000Z location UTC: parsing time "2024-10-29T19:15:52.011Z" as "2006-01-02T15:04:05.000000Z": cannot parse ".011Z" as ".000000"
ok  baz.libsonnet   903µs

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[x] I have updated the documentation accordingly.

jshlbrd commented 2 weeks ago

There are a few things needed to make this consistent across the CLI tool:

Any other references to test error and config error no longer make sense, it's better if they're all consistent one way or the other. (If they are going to reference the path in the config file, then it should be "transforms" with an s.)
This isn't obvious but it uses 4 spaces instead of \t\t for reporting info or errors, like this.
Port the same features and style to the playground command for consistency.

shellcromancer commented 2 weeks ago

Any other references to test error and config error no longer make sense, it's better if they're all consistent one way or the other. (If they are going to reference the path in the config file, then it should be "transforms" with an s.)

Updated the test output on errors for each place to make it more meaningful e.g. test condition error, or test config error

This isn't obvious but it uses 4 spaces instead of \t\t for reporting info or errors, like this.

Updated to use spaces.

Port the same features and style to the playground command for consistency.

Updated the playgrounds error handling.

jshlbrd commented 2 weeks ago

This is affecting the style of most of the CLI, so I pushed a commit to standardize it. You can try it with this config:


local sub = std.extVar('sub');

{
  tests: [
    {
      name: 'ex',
      transforms: [  // comment block for error.
        sub.tf.test.message({ value: 'brex.com' }),
        // sub.tf.net.domain.subdomain(),  // uncomment for runtime error.
        // sub.tf.net.domain.subdomain({ object: {source_key: 'test'}}),  // uncomment for vet error.
      ],
      condition: sub.cnd.str.eq({ value: '' }),  // comment for error.
    },
  ],
  transforms: [  // comment block for vet error.
    // sub.tf.net.domain.subdomain(),  // uncomment for runtime error.
    // sub.tf.net.domain.subdomain({ object: {source_key: 'test'}}),  // uncomment for vet error.
    sub.tf.send.stdout(),  // no-op.
  ],
}

shellcromancer commented 2 weeks ago

so I pushed a commit to standardize it.

Pushed changes look fine to me. Let's merge this one?

brexhq / substation