JUnit migration #7
Conversation
|
Review by @adriaanm. |
Most of the migration is straightforward. One exception is packrat3 (migrated to `test3` method). I simplified returned result by getting rid of useless Unit by changing this rule: repMany1(a,b) ~ not(b) into this rule: repMany1(a,b) <~ not(b) Also, I made all rules to be type safe. When constructing expected results, I decided to use scala.Symbol that allows us to use a bit more concise syntax for construction of one character strings. See `threeLists` method for details.
|
LGTM -- will be a lot easier to add more tests along these lines! |
| parseFailure("3start", 1) | ||
| parseFailure("-start", 1) | ||
| parseFailure("with-s", 5) | ||
| parseFailure("we♥scala", 3) |
There was a problem hiding this comment.
Is there a way to pass the enoding? I'm afraid relying on the default encoding can break tests on other platforms.
There was a problem hiding this comment.
You could use a unicode escape in the source here.
The unicode strings has been escaped using native2ascii -encoding utf8 a.txt b.txt command.
Extract common code that parses strings and extracts results into helper methods. In test1 and test2 I named the helper method `check` because it checks if expression-evaluating parser calculates the correct result. In test3, we are interested in parsing result (Success or Failure) thus helper methods are named `assertSuccess` and `assertFailure`.
|
@gkossakowski It's good, but I think it's valuable to test non-ASCII characters as well, (iff parser combinators support different encodings, of course). Anyway, you addressed my initial comment (platform-dependent test), so LGTM. |
|
Parser combinators will still be tested with non-ASCII characters as the Scala scanner will decode the unicode escapes. |
|
LGTM |
|
I manually verified that tests pass under: |
|
Merging, look forward to the rest of the tests! |
|
@retronym: haha! Good point! :-) I usually get it right when I write tests myself but I must admit migrating somebody's else code in such quantity is rather thankless job. |
|
I suppose this way saves on compile time :) |
|
@retronym I don't think it's the same thing, what I was aiming for is to test encoding support in parser combinators. I agree it's not straight-forward, nor a blocker for this PR. |
Indeed the studies have shown that being a Test Migrator is far inferior to being Harvey Weinstein, or God when it comes to gratitude. 😦 But let me be the one to say it: thank you! |
|
Hear, hear -- thanks, @gkossakowski! |
I've migrated more than half of all parser-combinators tests to JUnit.
Apart from 0e40547, the whole migration was straightforward because tests were already written in JUnit style.
I'm submitting progress on JUnit migration before I run out of steam.