MarkRoddy / brstest

Apache License 2.0
27 stars 8 forks source link

Roku's own unit testing framework beta #21

Open siekermantechnology opened 7 years ago

siekermantechnology commented 7 years ago

Mark, I'm quite curious what your opinion is of the BrightScript unit testing framework which Roku released very recently as a beta at https://github.com/rokudev/unit-testing-framework.

EnTerr commented 7 years ago

rokudev/unit-testing-framework seems based on this one (brsTest) - that's my opinion after i noticed multiple similarities (verbatim comments, verbatim diagnostic texts - including unconventional wording) while exploring one of the files.

What i am surprised about is that RokuCo has not given any attribution to this project here. Yes, it is Apache 2.0 but a credit is still due, right? The file itself says "Copyright (c) 2010 Mark Roddy" on top.

While the Co are free to borrow, copy, modify and do anything they please with this - they should mention that portions are taken from this project - is my understanding. Has this been licensed to them under another, no-attribution agreement?

6.5 years later, RokuCo re-invent the wheel by using BrsTest for parts, the result does not quite work - but they put their own exlusive license on it?!

MarkRoddy commented 7 years ago

I've walked through the Roku testing framework a bit at this point, though not super thoroughly, but I haven't seen anything that appears to be a very clear copy/paste of a significant chunk of code. If that's incorrect I'd love to know so I can make sure that all the people who have contributed to this library receive the credit they deserve. So if I am missing something I would love to see an explicit example of copied code pointed out.

EnTerr commented 7 years ago

@MarkRoddy - i don't know about "a significant chunk of code" but i have seen snippets which to me are "clear and convincing" evidence that Roku's U.T.F. has used brsTest as a starting point - even if it diverged, for better or worse. That makes it a "derivative work", does it not?

Now i don't think RokuCo intentionally skipped on crediting prior work - rather i think said library was outsourced to an Eastern European company and there is no high respect for copyright (esp. if it was something open source) - so it's almost certainly case of Hanlon's razor ("cock-up before conspiracy"). Othertimes Roku put fair effort to credit the open source they use, cue https://www.roku.com/separatelylicensedcode

EnTerr commented 7 years ago

Here is an example https://github.com/MarkRoddy/brstest/blob/7a6fdd6dcbf47dc20a632b66cddcd47f3548cfa2/source/brstest.brs#L279-L282]MarkRoddy/brstest//brstest.brs#L279-L282 vs https://github.com/rokudev/unit-testing-framework/blob/28d8154fa688cd5683035528ff014e005d2124eb/fw_baseTestSuite.brs#L116-L126]rokudev/unit-testing-framework//fw_baseTestSuite.brs#L116-L126

Notice the m.fail() invocation there is the only use of the fail() method - the BTS__Fail() (nee brstTcFail()) has become a no-op vestigial, so they stopped calling it in the other asserts

EnTerr commented 7 years ago

https://github.com/MarkRoddy/brstest/blob/7a6fdd6dcbf47dc20a632b66cddcd47f3548cfa2/source/brstest.brs#L286-L289]MarkRoddy/brstest//brstest.brs#L286-L289 vs https://github.com/rokudev/unit-testing-framework/blob/28d8154fa688cd5683035528ff014e005d2124eb/fw_baseTestSuite.brs#L132-L142]rokudev/unit-testing-framework//fw_baseTestSuite.brs#L132-L142

The msg = "Expression evaluates to false" in the latter is vestigial from the former. The meaning has shifted slightly, if written anew the correct wording in the 2nd case should be "Expression is not True" (since now it checks for two things - not Boolean and not True). Same applied to above, where "Expression evaluates to true" should be "Expression is not False" (since 2 ways to fail now)

EnTerr commented 7 years ago

https://github.com/MarkRoddy/brstest/blob/7a6fdd6dcbf47dc20a632b66cddcd47f3548cfa2/source/brstest.brs#L293-L298]MarkRoddy/brstest//brstest.brs#L293-L298 vs. https://github.com/rokudev/unit-testing-framework/blob/28d8154fa688cd5683035528ff014e005d2124eb/fw_baseTestSuite.brs#L148-L162]rokudev/unit-testing-framework//fw_baseTestSuite.brs#L148-L162 Note how the comment "Fail if the two objects are unequal as determined by the '<>' operator." uses "<>" to indicate not-equal operator but the message below uses "!=" - yes, in both places.

EnTerr commented 7 years ago

https://github.com/MarkRoddy/brstest/blob/7a6fdd6dcbf47dc20a632b66cddcd47f3548cfa2/source/brstest.brs#L302-L307]MarkRoddy/brstest//brstest.brs#L302-L307 vs https://github.com/rokudev/unit-testing-framework/blob/28d8154fa688cd5683035528ff014e005d2124eb/fw_baseTestSuite.brs#L170-L184]rokudev/unit-testing-framework//fw_baseTestSuite.brs#L170-L184

Ditto but this time the unusual wording is about "=" and "==". What are the odds of this happening by coincidence?

EnTerr commented 7 years ago

https://github.com/MarkRoddy/brstest/blob/7a6fdd6dcbf47dc20a632b66cddcd47f3548cfa2/source/brstest.brs#L311-L315]MarkRoddy/brstest//brstest.brs#L311-L315 vs https://github.com/rokudev/unit-testing-framework/blob/28d8154fa688cd5683035528ff014e005d2124eb/fw_baseTestSuite.brs#L192-L204]rokudev/unit-testing-framework//fw_baseTestSuite.brs#L192-L204

https://github.com/MarkRoddy/brstest/blob/7a6fdd6dcbf47dc20a632b66cddcd47f3548cfa2/source/brstest.brs#L319-L323]MarkRoddy/brstest//brstest.brs#L319-L323 vs https://github.com/rokudev/unit-testing-framework/blob/28d8154fa688cd5683035528ff014e005d2124eb/fw_baseTestSuite.brs#L212-L224]rokudev/unit-testing-framework//fw_baseTestSuite.brs#L212-L224