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

Option to override lowest level output call, eg DisplayProcessor.log(ignored, log) #129

Closed
johnjbarton opened this issue Apr 5, 2017 · 6 comments

Comments

@johnjbarton
Copy link
Contributor

ExecutionDisplay.log currently calls console.log(); I'd like to override it.

When running with the V8 node inspector Chrome Devtools, the Devtools Console shows lines similar to stdout. However, the Devtools Console does not understand colors so the lines look like:
�[31m- �[39m�[31mTypeError: Cannot read property 'xxx' of undefined�[39m

And stdout does not understand Chrome stacks like the Devtools, so by default the stacks printed by jasmine-spec-reporter are not the compact, linkified stack of the Devtools.

What would be neat is if I could add a CustomProcessor only when I detect the --inspector flags and in that CustomProcessor I would

  1. override displayFailedSpec() to console.error(spec.failedExpectations[0].stack);
    giving compact linkified stacks for interaction with devtools,
  2. override displayLog() to write directly to stdout without using console.log(), giving nice logs on stdout but avoiding semi-unreadable junk in the devtools console.

Would you consider at PR along these lines?

@bcaudan
Copy link
Owner

bcaudan commented Apr 6, 2017

Hi @johnjbarton!

If I understand correctly, you have two issues here:

  1. color flags are not interpreted by chrome console
  2. stacktrace are not displayed as errors

CustomProcessors are not designed to handle cases like these, I'd like to suggest another approach.

For the first issue, what about deactivate colors when --inspector flags is present?
For the second issue, what about trying to use console.error for stacktraces in all environments?

Does it sound good?

@johnjbarton
Copy link
Contributor Author

For the first issue, what about deactivate colors when --inspector flags is present?

Yes, I thought about that and perhaps it's ok. My team really likes jasmine-spec-reporter, so I was hoping to add support for devtools but leave the command line output as close to normal as feasible.

For the second issue, what about trying to use console.error for stacktraces in all environments?

Similarly, my goal is to keep the well-liked jasmine-spec-reporter output nice. The devtools has special 'linkifier' code to display the call stack from errors as file-paths-reduced-to-file-names with links to devtools Sources views. But if the console.error() is called in all environments, then stdout will have an un-linkified stack. In our case, we have very long paths with a machine-generated prefix, resulting in a lot of useless characters, so I use the jasmine-spec-reporter stacktrace option to clean this up.

To be sure, calling console.error() in displayFailedSpec() if --inspect is set does muck up the command line, but I don't see a way to avoid it. It's not so bad in practice because we are running tests, seeing nice green the command line then boom a red one, we switch attention to the devtools and see the nice stack linked to our issue. And if we don't have devtools running, the red has a nice compact stack trace.

CustomProcessors are not designed to handle cases like these,

I noticed this when I started a PR. I'll send a PR with separate option to see what you think.

@bcaudan
Copy link
Owner

bcaudan commented Apr 8, 2017

I was hoping to add support for devtools but leave the command line output as close to normal as feasible

With my current understanding, we can't have different output for devtools and command line, so output need to be compatible with both display or we need to choose our preferred display.

calling console.error() in displayFailedSpec() if --inspect is set does muck up the command line

How console.error muck up the command line? I have tried quickly and I have not seen any difference with console.log.

Nevertheless, if we want to have a nicer experience on devtools (and neglect the command line), instead of exposing only one low-level print option, we could expose two: one for common log and one for error log. By default, both would use console.log but it could be configurable for devtools.

Is it something that could be interesting?

@bcaudan
Copy link
Owner

bcaudan commented Apr 13, 2017

@johnjbarton any thoughts on that?

@johnjbarton
Copy link
Contributor Author

johnjbarton commented Apr 13, 2017

we can't have different output for devtools and command line,

Well we can, sorta: if you write directly to stdout, the devtools will not see the output. With the new print option in #130, jasmine's output can be sent to stdout and it will not appear in the devtools console.

instead of exposing only one low-level print option, we could expose two: one for common log and one for error log.

I kinda do that by using the print option for all jasmine output, then add a display processor with a single function having a side-effect of error output to the devtools console:

DisplayProcessor.prototype.displayFailedSpec = function(spec, log) {
  console.error(spec.failedExpectations[0].stack);
  return log;
};

This output is linkified by devtools. It does show up on the command line. I believe even this could be cleaned up by using the print option to output to the command line via a third file descriptor while redirecting stdout/err away. But that risks confusing failure modes when other errors are lost. As it stands, passing tests are great: nice jasmine-spec-reporter output and nothing in the console, while failing tests have linkified stacks in the devtools where I am focussing attention in that case.

I'll report back if I find additional limitations or issues.

@bcaudan
Copy link
Owner

bcaudan commented Apr 13, 2017

Great then!

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

No branches or pull requests

2 participants