Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cmd): log test errors #272

Merged
merged 3 commits into from
Nov 4, 2024

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
7 checks passed
@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