Skip to content

feat(cmd): log test errors#272

Merged
shellcromancer merged 3 commits intomainfrom
shellcromancer/feat/cli-tests-stderr
Nov 4, 2024
Merged

feat(cmd): log test errors#272
shellcromancer merged 3 commits intomainfrom
shellcromancer/feat/cli-tests-stderr

Conversation

@shellcromancer
Copy link
Contributor

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)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

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

@shellcromancer shellcromancer requested a review from a team as a code owner October 29, 2024 21:50
@shellcromancer shellcromancer force-pushed the shellcromancer/feat/cli-tests-stderr branch from 08540dd to 5230b1f Compare October 29, 2024 22:28
@jshlbrd
Copy link
Contributor

jshlbrd commented Oct 30, 2024

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 shellcromancer changed the title feat(cmd): log test transform errors to stderr feat(cmd): log test errors Oct 30, 2024
@shellcromancer
Copy link
Contributor Author

  • 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
Copy link
Contributor

jshlbrd commented Nov 3, 2024

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.
  ],
}

Copy link
Contributor Author

shellcromancer commented Nov 4, 2024

so I pushed a commit to standardize it.

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

@shellcromancer shellcromancer merged commit 917e29f into main Nov 4, 2024
@shellcromancer shellcromancer deleted the shellcromancer/feat/cli-tests-stderr branch November 4, 2024 22:38
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