SI-7265 General test for spec version #2666
Conversation
|
@som-snytt sorry for my naivety, but your comment about mima, considering the merge target of this branch, means pending/ contains tests to be activated once we cross a binary compatibility (and hence "leakyness") boundary, right ? |
|
@huitseeker That was my thinking, though I didn't expend massive amounts of brain power. Did I notice that the mima workflow includes whitelisting harmless changes? But I didn't want to cloud the review. My assumption on the unit test is that it shouldn't set properties because it might be run in-process with other tests. |
|
Valid wrt ScalaIDE PR validation |
|
Ah! The plot thickens. |
|
It would be nice to document which exceptions can be thrown, or better, pick one exception instead of potentially throwing multiple ones. Additionally, it would be nice to have more test cases with malicious input. |
|
@soc document as in throws? agreed. Scaladoc or annotation? Will a java app ever call this? But I was thinking we could throw |
|
@som-snytt I think scaladoc is enough. I think a |
|
OK, this revision guards the input version string (only) and scaladoc is updated to represent that only NumberFormat is thrown. |
|
@soc goaded me into updating the test, and I also simplified a bit. Soon the unit test will have to be a scalacheck. I'm starting to wish we'd left it for the cyborg progeny. I learned that s.substring(s.length,s.length) does not throw. Is that a good interview question? Maybe for separating the wheat from the brash. @huitseeker Your naivety is your most charming quality. |
The test for isJavaAtLeast uses the specification.version. The method argument must have the form "major.minor". The scaladoc is updated to reflect the new reality and a test is added under junit. Note that this implementation aims to be a simple compromise between the functional and imperative camps, that is, to be free of both closures and while loops. And to elicit no cruft like regexes and wrappers for strings. No doubt even more could be done in this department, but we don't wish to spoil the fun on codegolf.stackexchange.com. However, we might decide to sponsor a new site: codereviewpingpong.com For 2.10.x, javaSpecVersion is provided as a private member. The active test is under `run` and the `junit` test must bide its time in `pending`. For 2.11, the private members can be public and the app test replaced with the unit test.
|
What's the status on this PR? |
|
I think we're all duked out -- LGTM |
SI-7265 General test for spec version
5 participants
Review by @soc and @adriaanm