-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ScalaCheck specs #110
Add ScalaCheck specs #110
Conversation
There is an exception raised in Travis for Java 6 and 8:
Appears to be a problem related to forking the tests by sbt. I wonder if I'll need to disable forking, see sbt Reference Manual: Forking. It's not something I ever experienced locally.
|
we've seen similar-looking errors in the Scala build on Jenkins that are the result of a forked JVM crashing because it ran out of heap. I don't see anything about an OutOfMemoryError in the log, but it's still a hypothesis worth considering. you might try giving the forked JVM more heap is there a reason the tests run in a forked JVM? |
I created #112, and found out there is a reason why the JVM needs to be forked due to a classloader defect in SBT related to leaking libraries that are included with the compiler. I guess it's an issue for other scala-modules as well, including scala-parser-combinators. As suggested by @SethTisue, increasing the memory settings provided enough head room for the Travis builds to succeed. |
c89d8af
to
0786025
Compare
@biswanaths @ashawley fantastic, this looks amazing. I can't review line by line, but I guess we should just merge...? |
Let me go through this. |
needs rebase now. LGTM otherwise and I would suggest we merge in a few days, after the rebase, if there are no actual objections. @biswanaths, if you still need more review time on this, please speak up. |
0f38c67
to
c5ce6f5
Compare
I've rebased, and the Travis build succeeds. The property tests are only added the JVM side of the house for now. Good enough for the time being? I didn't realize #109 duplicated the tests between the two JS and JVM sides. |
after #133 is merged, will it be easy to make the ScalaCheck tests run on JS too? if not, I think it would be fine to merge this anyway, then create a new issue ("make ScalaCheck tests run on JS too") to at least document the situation and hopefully even attract a volunteer |
ready for merge, I think? |
94a8586
to
ea742d1
Compare
7d2a7d0
to
31c1fe9
Compare
I created this ScalaCheck test suite 2 years ago. At various points, I planned to merge it to trunk. However, I'm not sure how much of a great benefit it is. In the meantime, ScalaCheck development slowed down. In hindsight, this would have been a hinderance to bootstrapping the compiler (when that was the case) or making new builds of scala-xml for new Scala milestone releases. The ScalaCheck suite would have complicated things when Scala.js was added, as well. Most of the bugs in scala-xml had already been found and fixed over the dozen years of the code's existence. The ScalaCheck suite is useful as a tracking branch to help learn the impact of new changes and fixes, or to verify there weren't accidental regressions, but I wouldn't want to impose on another maintainer or to contributors. If scala-xml gets a re-write, maybe it would become useful. I'm going to push it of to an imaginary 2.0 release, for now. |
if the ScalaCheck tests were confined to their own subproject, then there would be no issue, I think, the bootstrap would just avoid doing anything with that subproject
again, I think a subproject would solve this. since it would leave us free to run the ScalaCheck tests when a ScalaCheck build is available and if not, not |
Adds 26 specifications in ScalaCheck, with 154 properties, of 18 are proofs by example, and the rest are 136 tests using randomly-generated values. ScalaCheck requires at least 100 generated tests to pass, by default. So that gives 13,618 new tests! The suite should be a real benefit for future maintenance endeavors, rewrites, documentation, or finishing a Scala-based parser to avoid depending on the the Xerces/SAX Java library. These integrate well with the existing suite of JUnit 4 tests. If you want to run both the ScalaCheck and JUnit tests from SBT, > test ... [info] Passed: Total 145, Failed 0, Errors 0, Passed 145 [success] Total time: 43 s, completed Sep 30, 2016 2:19:56 PM If you want to run a single spec, > testOnly scala.xml.XMLSpec [info] + XML.write: OK, passed 100 tests. [info] Passed: Total 1, Failed 0, Errors 0, Passed 1 [success] Total time: 3 s, completed Sep 30, 2016 2:20:28 PM If you want to run all the ScalaCheck specs, > testOnly *Spec ... [info] Passed: Total 26, Failed 0, Errors 0, Passed 26 [success] Total time: 30 s, completed Sep 30, 2016 2:22:58 PM If you want to run the specs under a namespace, > testOnly scala.xml.dtd.*Spec ... [info] + dtd.DTD.toString: OK, passed 100 tests. [info] + dtd.DocType.new(name): OK, passed 100 tests. [info] + dtd.DocType.new(name, extId, intSubset): OK, passed 100 tests. [info] + dtd.DocType.new(name, extId, emptyInt): OK, passed 100 tests. [info] + dtd.DocType.toString: OK, passed 100 tests. [info] Passed: Total 4, Failed 0, Errors 0, Passed 4 [success] Total time: 6 s, completed Sep 30, 2016 3:14:05 PM If you want to generate 1000 tests instead of 100, > testOnly *Spec -- -s 1000 ... [info] Passed: Total 26, Failed 0, Errors 0, Passed 26 [success] Total time: 217 s, completed Sep 30, 2016 2:27:31 PM If you want to generate 10 tests, instead of 100, > testOnly *Spec -- -s 10 ... [info] Passed: Total 26, Failed 0, Errors 0, Passed 26 [success] Total time: 10 s, completed Sep 30, 2016 2:23:49 PM If you want to generate full back traces on errors and display the elapsed time for each property, > testOnly *Spec -- -verbosity 2 ... [info] + parsing.XhtmlParser.initialize: OK, passed 100 tests. [info] Elapsed time: 0.046 sec [info] + parsing.XhtmlParser.prolog: OK, passed 100 tests. [info] Elapsed time: 0.022 sec [info] + parsing.XhtmlParser.document: OK, passed 100 tests. [info] Elapsed time: 0.015 sec [info] Passed: Total 26, Failed 0, Errors 0, Passed 26 [success] Total time: 30 s, completed Sep 30, 2016 2:29:24 PM The default verbosity is 0, > testOnly *Spec -- -verbosity 0 ... [info] + parsing.XhtmlParser.initialize: OK, passed 100 tests. [info] + parsing.XhtmlParser.prolog: OK, passed 100 tests. [info] + parsing.XhtmlParser.document: OK, passed 100 tests. [info] Passed: Total 26, Failed 0, Errors 0, Passed 26 [success] Total time: 26 s, completed Sep 30, 2016 2:30:26 PM To run only the failed tests, > testQuick [info] Passed: Total 0, Failed 0, Errors 0, Passed 0 [info] No tests to run for test:testQuick [success] Total time: 1 s, completed Sep 30, 2016 2:32:50 PM Currently, these tests in ScalaCheck will cover only 25% of the source code, and 20% of branches. Not surprisingly, there are some defects in scala-xml for certain types and classes. I am forced to comment out and disable those tests and generators so that the suite passes. Those issues should be taken up separately and in subsequent pull requests. When those tests and generators are re-enabled for those types, the code coverage would most likely increase. There are no shrink heuristics defined written to help ScalaCheck identify the minimum value to falsify a property of an XML string or data structure. This will be a nice to have, but was too advanced for me to take on at this time. Right now, the generators equally weight the Node types among those that are allowed. It would probably be worthwhile to have the frequencies be more realistic. For example, have a a greater emphasis on Elem and EntityRef elements over Comment, ProcInstr, nulls, empty strings and empty lists. The frequencies should be closer in line with what a typical XML file would be, but also more broadly cover the code and sooner using fewer generated values. Similarly, there is not an appropriate sizing for lists. Presently, ScalaCheck randomly selects a number, and then the generators I wrote will try to approach termination by recursively halving and square-rooting at each level of descent.
Exception thrown was made more consistent in 389cdce
Name-based pattern matching dropped in 2.13.0-M5
Exception thrown was made more consistent in 9a0db27
@ashawley was looking at the open PRs and noticed this. is this something you might like to return to yourself? or should we seek an open-source volunteer to adopt it? (or should we just close the PR and not worry about it?) |
closing for inactivity. perhaps a volunteer would like to revive it |
Adds 26 specifications in ScalaCheck, containing 154 properties, of 18 are proofs by example, and the rest are 136 tests using randomly-generated values. ScalaCheck requires at least 100 randomly-generated tests to pass, by default. So that gives 13,618 new tests!
The suite should be a real benefit for future maintenance endeavors, rewrites, documenting the code, or finishing a Scala-based parser to avoid depending on the the Xerces/SAX Java library.
These integrate well with the existing suite of JUnit 4 tests. If you want to run both the ScalaCheck and JUnit tests from SBT,
If you want to run a single spec,
If you want to run all the ScalaCheck specs,
If you want to run the specs under a namespace,
If you want to generate 1000 tests instead of 100,
If you want to generate 10 tests, instead of 100,
If you want to generate full back traces on errors and display the elapsed time for each property,
The default verbosity is
0
,To run only the failed tests,
Currently, these tests in ScalaCheck will cover only 25% of the source code, and 20% of branches.
Not surprisingly, there are some defects in scala-xml for certain types and classes. I am forced to comment out and disable those tests and generators so that the suite passes. Those issues should be taken up separately and in subsequent pull requests. When those tests and generators are re-enabled for those types, the code coverage would most likely increase.
There are no shrink heuristics defined written to help ScalaCheck identify the minimum value to falsify a property of an XML string or data structure. This will be a nice to have, but was too advanced for me to take on at this time.
Right now, the generators equally weight the
Node
types among those that are allowed. It would probably be worthwhile to have the frequencies be more realistic. For example, have a a greater emphasis onElem
andEntityRef
elements overComment
,ProcInstr
, nulls, empty strings and empty lists. The frequencies should be closer in line with what a typical XML file would be, but also cover the code more broadly and sooner in fewer tests.Similarly, there is not an appropriate sizing for lists. Presently, ScalaCheck randomly selects a number, and then the generators I wrote will try to approach termination by recursively halving and square-rooting at each level of descent.