feat(reporter): implement Jest reporter by lkipke · Pull Request #80 · hulu/roca

lkipke · 2021-03-17T20:01:29Z

Summary

This leverages the jest packages and tap-parser to create a Jest-style reporter.

Screenshots

Default reporter

Verbose reporter

Failing due to assert

Failing due to runtime exception

In test case

In source code

Print statements

Code:

m.it("passing test case", sub()
    print "here's a print statement with an object: " { some: "field" }
    m.assert.equal("abcd", "abcd", "they are equal")
end sub)

m.it("passing test case", sub()
    print "hello, i am but a humble print statement"
    m.assert.deepEquals({ foo: "abcd" }, { foo: "abcd" }, "they are equal")
end sub)

lkipke · 2021-03-17T20:13:35Z

+        // Flatten console output because tap-parser splits output by newline in the "extra" event.
+        // We don't know which lines are supposed to go together, so just print them all together.
+        if (this.currentResults.console) {
+            this.currentResults.console = [


This was the best solution I could think of. It's a little wonky, but it's not too far off from how Jest prints console.log statements in JS tests. The main problem is we don't have any stack trace for the logs.

Jest reporter for roca (what this PR implements):

Actual jest reporter for JS (I put some console.logs in the roca project's JS test suite):

I think this is fine as long as we get stack traces for errors which seems like we do already 😄

Agreed! Since we treat brs (specifically its print statements) as an opaque box from the roca perspective, I'm fine with losing that for now. It might be possible to add an eventEmitter that handles writes to stdout instead of calling interpreter.stdout.write() directly from within brs's implementation of print, but that's not necessary here at all. It'd be neat to consider for the future though — especially in combination with a --quiet in brs to suppress writes to stdout?

It'd be neat to consider for the future though — especially in combination with a --quiet in brs to suppress writes to stdout?

I agree, that'd be great!

alimnios72 · 2021-03-18T23:27:48Z

+        // Flatten console output because tap-parser splits output by newline in the "extra" event.
+        // We don't know which lines are supposed to go together, so just print them all together.
+        if (this.currentResults.console) {
+            this.currentResults.console = [


I think this is fine as long as we get stack traces for errors which seems like we do already 😄

alimnios72

Thanks for the changes

sjbarag

For safety's sake it might be worth keeping --reporter spec as the default for a release or two while we opt-into --reporter jest to shake out any hidden bugs, but if you're comfortable with it I'm down to give this a try!

Excellent work @lkipke — I'm excited to get this out to the world!

sjbarag · 2021-03-25T23:37:51Z

+                        .map((entry) => entry.message)
+                        .join(""),
+                    origin: "", // TODO: figure out how to get the stack trace
+                    type: "print" as any, // a little hack because brightscript uses `print` instead of `console.log`


Aw bummer it reports as console.print instead of just print? So close!

I know, so close 😢 And we have no control over the outputted string here, either, so we can't use replace or anything, sadly.

lkipke added 13 commits March 5, 2021 14:24

feat(reporter): implement jest reporter

fe07d29

clean up after rebase

fa8dc56

clean up more

dfe14a6

clean up

6f448ac

add back run complete

ad3feca

remove unnecessary changes

2c567f8

remove changes

a1fea64

remove unnecessary changes

ebfd536

clean up

5b4431f

clean up exec errors

48a3774

comments

0e33690

parse expected/wanted

221c4b1

remove suite names

02cc0cf

lkipke added the enhancement New feature or request label Mar 17, 2021

lkipke requested review from alimnios72 and sjbarag March 17, 2021 20:01

lkipke self-assigned this Mar 17, 2021

use word print instead of log

6157788

lkipke commented Mar 17, 2021

View reviewed changes

use relative paths

1f015e4

alimnios72 reviewed Mar 18, 2021

View reviewed changes

pr feedback

220cd83

alimnios72 approved these changes Mar 19, 2021

View reviewed changes

lkipke added 2 commits March 19, 2021 14:10

start unit tests

1ee71bb

start unit tests

6d0895f

sjbarag approved these changes Mar 25, 2021

View reviewed changes

lkipke added 3 commits March 29, 2021 13:49

pr feedback

ef6bdcd

remove transform

965a3a0

fix tests

58715a5

lkipke merged commit 49161c7 into hulu:main Mar 29, 2021

lkipke deleted the implement-jest-reporter branch March 29, 2021 21:21

Uh oh!

Conversation

lkipke commented Mar 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Screenshots

Default reporter

Verbose reporter

Failing due to assert

Failing due to runtime exception

In test case

In source code

Print statements

Uh oh!

Uh oh!

lkipke Mar 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alimnios72 Mar 18, 2021

Choose a reason for hiding this comment

Uh oh!

sjbarag Mar 25, 2021

Choose a reason for hiding this comment

Uh oh!

lkipke Mar 29, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alimnios72 Mar 18, 2021

Choose a reason for hiding this comment

Uh oh!

alimnios72 left a comment

Choose a reason for hiding this comment

Uh oh!

sjbarag left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sjbarag Mar 25, 2021

Choose a reason for hiding this comment

Uh oh!

lkipke Mar 29, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lkipke commented Mar 17, 2021 •

edited

Loading

lkipke Mar 17, 2021 •

edited

Loading