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

Allow less verbose output #2976

Closed
4 tasks done
johtso opened this issue Mar 7, 2023 · 9 comments · Fixed by #5917
Closed
4 tasks done

Allow less verbose output #2976

johtso opened this issue Mar 7, 2023 · 9 comments · Fixed by #5917
Labels
discussion p2-nice-to-have Not breaking anything but nice to have (priority)

Comments

@johtso
Copy link

johtso commented Mar 7, 2023

Clear and concise description of the problem

Currently, if a test fails, I see the error a total of 3 times, one time being a complete serialized representation of the error.

The most problematic of these is the serialized error.

In my use-case, I happen to be using Zod who's Error objects can be very large.

When every test run output ends with a serialized representation of a Zod error, I end up having to scroll up a page or more until I can see the normal vitest output.

This is what the output looks like for me:
Screenshot_2023-03-07 21 18 53_AUmwcy

Suggested solution

Would it make sense to have a flag making the output less noisy?

This seems to be the offending line:

logger.error(c.red(c.bold('Serialized Error:')), c.gray(propertiesString))

Alternative

No response

Additional context

No response

Validations

@sheremet-va
Copy link
Member

I see the error a total of 3 times

Provide a reproduction for this claim please. Current solution is working correctly and is compatible with how Node.ja prints errors.

@johtso
Copy link
Author

johtso commented Mar 7, 2023

@sheremet-va See screenshot below. The error.message is shown once under "DEV", once under "FAIL", and then the entire serialized error is shown at the end.

Current solution is working correctly and is compatible with how Node.js prints errors

I'm not saying this is a bug, I'm saying that the current logging behaviour is making it very awkward to use, and I can't find any existing functionality allowing the user to limit the verbosity. My only solution was to jump into node_modules and comment out the line.

image

@Farnsi
Copy link

Farnsi commented May 16, 2023

I think you have to write your own reporter.

Currently i use a very simple one like this:

import c from 'picocolors'

import type { UserConsoleLog, Vitest } from 'vitest'

export default class VerboseReporter {
  ctx!: Vitest

  onInit(ctx: Vitest): void {
    this.ctx = ctx
  }

  async onFinished(files = this.ctx.state.getFiles()) {
    const logger = this.ctx.logger
    for (const file of files) {
      for (const task of file.tasks) {
        if (task.result) {
          if (task.result.state === 'pass') {
            logger.log(c.gray(file.name), c.green('success'), task.name)
          } else {
            logger.error(c.gray(file.name), c.red('failed '), task.name)
          }
        }
      }
    }
  }

  onUserConsoleLog(log: UserConsoleLog) {
    process[log.type].write(log.content)
  }
}

But it has no good error reporting (i manage errors in my app).

Had only a quick look at the vitest reporters and i think they are not good to extend. @devs export the reporters.

@sheremet-va sheremet-va added the p2-to-be-discussed Enhancement under consideration (priority) label Feb 12, 2024
@leanne1
Copy link

leanne1 commented Mar 25, 2024

I use Nock and if nock fails to match a request the serialized output across a few failing tests can be so huge it can take half a minute to print it out and it cripples the terminal and i have to close the tab. I would also like to see flag to make this optional.

@hi-ogawa
Copy link
Contributor

We could pass truncate option for inspect(errorProperties, { truncate: chaiConfig.truncateThreshold }), but I'm not sure if we should reuse current default 40 truncateThreashold as it gets too small.

const propertiesString = inspect(errorProperties)
logger.error(c.red(c.bold('Serialized Error:')), c.gray(propertiesString))

I think I understand the annoyance of huge inspect output, but no one has provided a reproduction for such worst case yet. It always helps to have a runnable example, so that we can understand and evaluate the issue accurately.

@timovv
Copy link

timovv commented May 16, 2024

Just wanted to add another perspective here: we have a use case where the Error object that gets thrown includes information about the HTTP request that was made that caused the error. This request object can include sensitive information such an access token, which we really don't want showing up in our CI logs. The normal Error toString implementation doesn't show the whole content of the Error object, but this serialized error output dumps the whole content of the object to console, including the sensitive information.

We are looking for a way to suppress this verbose Serialized Error output in our pipelines. Some kind of option to disable that would be fantastic.

(Cc @mpodwysocki)

@sheremet-va
Copy link
Member

Did you try Vitest 1.6.0?

@timovv
Copy link

timovv commented May 16, 2024

Yeah, I am using 1.6.0. I just saw in the CHANGELOG the new functionality of using toJSON to serialize the error, which I assume is what you are referring to? We will investigate overriding that.

@sheremet-va
Copy link
Member

Yeah, I am using 1.6.0. I just saw in the CHANGELOG the new functionality of using toJSON to serialize the error, which I assume is what you are referring to? We will investigate overriding that.

Yes, we iterate over returned properties now (if it's an object)

@sheremet-va sheremet-va added p2-nice-to-have Not breaking anything but nice to have (priority) and removed p2-to-be-discussed Enhancement under consideration (priority) labels Jun 18, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discussion p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants