Allow custom test reporter

[luis-soares-sky]

This PR adds a new way to specify custom test reporters. This is extremely useful when developers need to access and export unit test results towards a custom pipeline.

// bsconfig.json
{
    "rooibos": {
        "reporters": [
            // Specify any of the built-in reporters...
            "console",
            "junit",
            // ...and/or point to your own factory.
            "MyNamespace_MyCustomReporter"
        ]
    }
}

' MyCustomReporter.bs
namespace MyNamespace
    class MyCustomReporter extends rooibos.BaseTestReporter

        override function onBegin()
            ' Do something just before the test suites run...
        end function

        override function onEnd(allStats)
            ' And do something else when they all end!
            '  * Like an HTTP call to report the results to a node-based server...
            '  * Or pipe data through a socket connection...
            '  * Anything, really.
        end function

    end class
end namespace

Notes:

• I opted for a new field reporterClass that isn't wrapped in a string, primarily because I didn't want to break existing behaviour, and reporter already existed.
  • This could introduce problems for people who don't know what they're doing, and there's probably gotchas (i.e. namespacing shenanigans), but maybe it's something we could clarify and document?
  • I added a new field reporters, as specified in Proposal: Reporter support #268.
  • reporter is now considered deprecated, and its value will be appended to the final reporters list.
• I've added a safeguard to only execute each of the factories if it's actually a function. This helps immensely since I don't quite know yet how to import said class to a node-based test suite.
  • The only solution I can think of right now, is to add some way of defining a new import for RooibosSession.createNodeFile via config, or potentially trying to find the file that the class belongs to.
  • I know how to do the former, but I'm not sure how clean that would be. And the latter is a bit of a mystery for me right now. If it's a requirement for this PR and anyone can help me out, that would be appreciated.

[luis-soares-sky]

This PR sort of acts as a follow-up to #212 (review). When we try to use the upstream version of Rooibos v5 with the script in that PR's linked comment, we come across an issue where we can't access the test results via the scene, because it is GC'd once Rooibos_init execution is complete.

We've been flying by with a custom fork that hacks around it, by declaring both scene and screen in m, but that solution doesn't seem right. This PR is kind of the alternative to that.

While the node-side script would stay pretty much the same, the Roku-side Main.brs example I provided would look slightly different:

' Main.bs

sub main()
    Rooibos_init()
    sleep(5000) ' Give time for the debug protocol to actually receive the data in VSC's debug console.
end sub

class SocketTestReporter extends rooibos.BaseTestReporter
    override sub onBegin()
        m.waitOnSocket()
    end sub

    override sub onEnd(allStats as rooibos.Stats)
        if m.connection <> invalid
            ' Build the test report and convert it to JSON for sending through the socket connection.
            report = m.buildReport(allStats, m.testRunner.testSuites)
            reportJSON = formatJson(report)

            ' We need to invalidate the message port, so that sendStr(...) becomes synchronous and blocks the thread while
            ' sending the JSON. Otherwise, the app will shut down, potentially cutting off the JSON string.
            m.connection.setMessagePort(invalid)
            m.connection.sendStr(reportJSON)
        end if
    end sub

    private sub waitOnSocket()
        port = createObject("roAppInfo").getValue("rooibos_port")
        if NOT port <> "" then return ' NOT <> "" because it can either be empty or invalid, and this way's easier to write.

        m.socketMessagePort = createObject("roMessagePort")
        m.socket = createObject("roStreamSocket")
        m.socket.setMessagePort(m.socketMessagePort)
        addr = createObject("roSocketAddress")
        addr.setPort(port.toInt())
        m.socket.setAddress(addr)
        m.socket.notifyReadable(true)
        x = m.socket.listen(1) ' bs:disable-line:LINT1005

        if NOT m.socket.eOK()
            print `[ROOIBOS-V5]: Could not create socket.` ' bypass-checker
            return
        end if

        print `[ROOIBOS-V5]: Waiting for CI socket connection on port: ${port}` ' bypass-checker
        while true
            msg = wait(1, m.socketMessagePort)
            if type(msg) = "roSocketEvent"
                if m.socket.isReadable()
                    newConnection = m.socket.accept()
                    if newConnection <> invalid
                        print `[ROOIBOS-V5]:${port} connected! Running tests...` ' bypass-checker
                        m.connection = newConnection
                        return
                    end if
                    print "[ROOIBOS-V5]: Socket connection failed" ' bypass-checker
                else
                    if newConnection <> invalid AND NOT newConnection.eOK()
                        print `[ROOIBOS-V5]: Closing connection on port: ${port}` ' bypass-checker
                        newConnection.close()
                    end if
                end if
            end if
        end while
    end sub

    private function buildReport(stats as rooibos.Stats, testSuites as Object) as Object
        report = {}
        report["success"] = NOT stats.hasFailures
        report["totalTestCount"] = stats.ranCount
        report["failedTestCount"] = stats.failedCount
        report["tests"] = []

        for each testSuite in testSuites
            for each group in testSuite.groups
                for each test in group.tests
                    testResult = {}
                    testResult["isFail"] = test.result.isFail
                    testResult["name"] = test.name
                    testResult["message"] = test.result.message
                    testResult["filePath"] = `file://${testSuite.filePath.trim()}:${stri(test.lineNumber).trim()}`
                    report.tests.push(testResult)
                end for
            end for
        end for

        return report
    end function
end class

// bsconfig.json
{
    ...,
    "rooibos": {
        ...,
        "reporters": [
            "console",
            "SocketTestReporter"
        ]
    }
}

[georgejecook]

@chrisdp I'm happy to merge this without node class support, and we can add that later. @luis-soares-sky - Can we document that node classes are not supported at this time, please?

[luis-soares-sky]

@georgejecook: I added a note in the docs. Can you check if they're acceptable? I also opened an issue to track adding node class support: #266

[TwitchBronBron]

I'd like to take a few days to think over this and discuss with @chrisdp. There are no changes to request right now, but I'll change my review or update my requests when we've had a chance to review it.

[georgejecook]

I'm good in principal, @luis-soares-sky ; but we'll give Chris and Bron time to have a look.

[TwitchBronBron]

We are hoping to discuss this tomorrow (Friday) morning.

[TwitchBronBron]

Okay, so Chris and I came up with a more formal proposal around this, which you can read here. Since we're opening this concept of "custom reporters" up to general use, we wanted to be intentional about public API of the reporter.

We obviously don't expect you to implement all of that proposal, so to keep this PR focused, here's what I think would be the minimum to align this PR:

1. deprecate reporter and define a new property called reporters (and add any value from .reporter to reporters internally when processing to support backwards compatibility)
2. remove the customReporter as it's replaced by reporters
3. update rooibos to construct ALL reporters found in the reporters list at startup
4. update rooibos to, for every reporter, call reporter.onBegin and reporter.onEnd at the start and the end of the suite and pass in the data you were going to pass to customReporter.
5. align the different JUnit and Console reporters to this new spec
6. make sure that the default reporters are still used when reporters[] is not defined.

@georgejecook, @luis-soares-sky we'd love to know your thoughts on this proposal. :)

[luis-soares-sky]

Steps 1, 2, 3 and 6 are now implemented.

I've not made the API changes yet (steps 4 and 5), since I'm waiting until the API spec is a bit more fleshed out.

[luis-soares-sky]

@georgejecook @TwitchBronBron @chrisdp I've now updated the PR to conform to all proposed steps, and updated both the description and provided examples.

[luis-soares-sky]

@georgejecook @TwitchBronBron @chrisdp any news on this?

[TwitchBronBron]

Hey, just a few small suggestions.

[luis-soares-sky]

Hey, is there any news on this? Any further changes I need to make?

[TwitchBronBron]

Hey @luis-soares-sky I'm so sorry, we've been swamped the past few weeks. I'll try and fit in our final review next week.

[TwitchBronBron]

This looks good to me. Thanks for your work on this. @georgejecook , can you cut a new release with these changes?