search

hulu / roca

A command-line tool for running brightscript tests
Apache License 2.0
25 stars 19 forks source link

feat(runner): Convert test runner from BrightScript to TS #62

Closed lkipke closed 3 years ago

lkipke commented 3 years ago

Change Summary

This converts the functionality from roca_main.brs to Typescript, using the new createExecuteFromScope functionality implemented in https://github.com/sjbarag/brs/pull/603.

Running tests in TS rather than Brightscript gets us much more flexibility with error handling/reporting, plus it allows for much easier future enhancements to CLI args (think: passing in a single file name to test, grep-ing file patterns, etc). And it will integrate nicely with a Jest-style reporter that we will eventually (hopefully) have.

lkipke commented 3 years ago

This will definitely give us more flexibility in the future, thanks for the hard work here. Other than my comments below I have a question regarding tap.brs: can we convert that file to Typescript in the future? if so, is there any issue tacking that?

I don't think we can convert tap.brs, because each brightscript function calls into it to report test results.

sjbarag commented 3 years ago

This will definitely give us more flexibility in the future, thanks for the hard work here. Other than my comments below I have a question regarding tap.brs: can we convert that file to Typescript in the future? if so, is there any issue tacking that?

I don't think we can convert tap.brs, because each brightscript function calls into it to report test results.

That sounds correct! The only alternative is to replace TAP as the brightscript-javascript bridge with some other format of our choosing. I could see a world where it becomes a JSON payload (with lines prefixed by some magic string for easier tagging), or if we wanted to get really creative we could add an extra function to the global _brs_ namespace that directly interacts with the javascript side — maybe with a node event-emitter that roca subscribes to?

That kind of migration seems out of scope for now though, and might not even be necessary if TAP isn't in the way of implementing features.