AllTests.java causes all tests to run twice #319
Comments
drywolf
changed the title
|
:-)
I was wondering if someone else would notice that. The only reason is
because I use AllTests in the Eclipse IDE to run all the tests. I wonder if
we rename AllTests to not include the name "tests" then Maven wouldn't pick
it up as a test class, and it wouldn't get run. I'm not sure how we
guarantee that the V8RuntimeNotLoadedTest runs first in Maven, but so far
it "appears" to work.
…On Thu, 27 Jul 2017 at 07:36 Wolfgang Steiner ***@***.***> wrote:
For an upcoming PR of mine I have been writing / rewriting some unit
tests, it was then that I realized that just invoking the usual mvn test
command runs all J2V8 unit-test twice.
This is caused because AllTests.java
<https://github.com/eclipsesource/J2V8/blob/master/src/test/java/com/eclipsesource/v8/AllTests.java#L36>
does declare a test suite that contains *all* of the remaining unit tests
in the J2V8 code.
This does fully *double* the time it takes to run the J2V8 unit tests, so
if it could have taken 5 minutes it does now always take 10 minutes. I'm
feeling this a lot, since for the PR that I am working on I have to
sometimes repeatedly run the entire J2V8 build and tests for
all the platforms, all the architectures, all the virtual build
environments that I can run on my environment, which are currently about
8 runs of the unit tests if I don't explicitely exclude them for a
particular run.
@irbull <https://github.com/irbull> My question here is, if this is
absolutely necessary and done by design or if (as I understand it) it just
tries to make sure that the V8RuntimeNotLoadedTest is run before any
other test in the package. (at least this is what the code comment tells)
// V8RuntimeNotLoadedTest must be run first. This is because we need to test when the natives are not loaded
// and once the V8 class is loaded we cannot unload it.
Because then I think it would just be enough to include *only* the
V8RuntimeNotLoadedTest in the suite and just make sure that the suite
runs before any of the other tests (which it seems to reliably do).
@irbull <https://github.com/irbull> Please let me know what your take is
on this ... I would include potential changes to the code to improve this
in my upcoming PR then. Thanks 😉
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#319>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAMBE_H-ZOaFj6_Q-pdZdNDqOHxfUYQwks5sSKBwgaJpZM4OlYA->
.
|
|
Thanks for the quick reply. On a related note ... the I was unable to come up with a working alternative implementation of the class-loading part on Android. As a workaround I would instead add some code to skip this test on Android for now if this is ok to you @irbull ? |
|
Yep, sounds good. Any tests on Android are better than what we have now. V8RuntimeNotLoaded is not really great test anyways. If things are not loaded, we'll know pretty quickly. |
|
I just downloaded Eclipse Oxygen (4.7.0), and with the code in AllTests.java commented out I was still able to run all unit tests fine ... right-click on src/test/java > Run As > JUnit Test ... same number of tests run and same results, just like when I invoke them from the command line via @irbull Is there some alternative integration or additional feature in Eclipse that you are using that still
|
|
No, I don't think so. I just deleted the AllTests.java and did what you said, and it seems to run fine too (I'm trying to track down some threading issues, so some tests failed, but I don't think that has anything to do with removing AllTests.java). I'm fine with just removing it. |
|
Ok, I think if we want to keep running
On a related note, I am now investigating the failing Android tests and I just realized that some of the tests are crashing the Android Activity that is used to run the junit tests in the Android emulator. As a result the entire test run was aborted early and did just run a small percentage of the tests up to this point. The problem seems to happen in tests that are throwing java exceptions for the most part (those result in a crash in the native code) Also some of the threading / locker tests seem to result in deadlocks and hit the test timeout limit (10 minutes). I will post some updates about those failed tests a little bit later, right now I'm more focused on getting the last features that I have on my roadmap into a suitable form for the PR. |
|
All tests has been removed. Closing. |
For an upcoming PR of mine I have been writing / rewriting some unit tests, it was then that I realized that just invoking the usual
mvn testcommand runs all J2V8 unit-test twice.This is happening because AllTests.java does declare a test suite that contains all of the remaining unit tests in the J2V8 code.
This does fully double the time it takes to run the J2V8 unit tests, so if it could have taken 5 minutes it does now always take 10 minutes. I'm feeling this a lot, since for the PR that I am working on I have to sometimes repeatedly run the entire J2V8 build and tests for
all the platforms, all the architectures, all the virtual build environmentsthat I can run on my environment, which are currently about 8 runs of the unit tests if I don't explicitely exclude them for a particular run.@irbull My question here is, if this is absolutely necessary and done by design or if (as I understand) it just tries to make sure that the
V8RuntimeNotLoadedTestis run before any other test in the package. (at least this is what the code comment tells)Because then I think it would just be enough to include only the
V8RuntimeNotLoadedTestin the suite and just make sure that the suite runs before any of the other tests (which it seems to reliably do).@irbull Please let me know what your take is on this ... I would include potential changes to the code to improve this in my upcoming PR then. Thanks 😉
The text was updated successfully, but these errors were encountered: