lagom / online-auction-scala

Other
200 stars 128 forks source link

Tests can be contaminated by manual test data #31

Open TimMoore opened 7 years ago

TimMoore commented 7 years ago

Reproduction

  1. Start the sample app with a fresh database by running sbt clean runAll
  2. Create a user and an item
  3. Stop the server
  4. sbt test

Expected behavior

The tests should pass.

Actual behavior

[info] The Item service
[info] - should allow creating items
[info] - should return all items for a given user
[info] - should should emit auction started event *** FAILED ***
[info]   ItemUpdated(5d50b8b0-022b-11e7-87c6-bf22b4dc5f33,51971f4d-da2e-4c8d-a758-c047404df29a,title,description,USD,Created) was not an instance of com.example.auction.item.api.AuctionStarted, but an instance of com.example.auction.item.api.ItemUpdated (ItemServiceImplIntegrationTest.scala:87)

The events created by the test are not isolated in any way.

Workaround

sbt clean test

TimMoore commented 7 years ago

This has caused some failures in Travis CI as well: https://travis-ci.org/lagom/online-auction-scala/builds/208863419

ignasi35 commented 7 years ago

@TimMoore do you think this is also affecting the java version? (but we've never seen it for some exec order of the tests?)

TimMoore commented 7 years ago

The Java tests pass, but with an error logged:

[info] Test com.example.auction.item.impl.ItemServiceImplIntegrationTest.shouldEmitItemUpdatedEvent started
[error] item - Exception in RestCallId{method=PUT, pathPattern='/api/item/:id'}
com.lightbend.lagom.javadsl.api.transport.TransportException: Can't update an item of a completed Auction.
    at com.example.auction.item.impl.ItemServiceImpl.lambda$null$3(ItemServiceImpl.java:105) ~[classes/:na]
    at java.util.concurrent.CompletableFuture.uniHandle(CompletableFuture.java:822) ~[na:1.8.0_112]
    at java.util.concurrent.CompletableFuture$UniHandle.tryFire(CompletableFuture.java:797) ~[na:1.8.0_112]
    at java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:443) ~[na:1.8.0_112]
    at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289) ~[na:1.8.0_112]
    at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056) ~[na:1.8.0_112]
    at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692) ~[na:1.8.0_112]
    at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157) ~[na:1.8.0_112]
TimMoore commented 7 years ago

That error is logged with clean data too, though, so it must be unrelated to this issue.

ignasi35 commented 7 years ago

I think that stacktrace is expected:

[info] Test com.example.auction.item.impl.ItemServiceImplIntegrationTest.shouldFailEditOnBiddingFinished started
[info] Test com.example.auction.item.impl.ItemServiceImplIntegrationTest.shouldEmitItemUpdatedEvent started
[error] item - Exception in RestCallId{method=PUT, pathPattern='/api/item/:id'}
com.lightbend.lagom.javadsl.api.transport.TransportException: Can't update an item of a completed Auction.
        at com.example.auction.item.impl.ItemServiceImpl.lambda$null$3(ItemServiceImpl.java:105) ~[classes/:na]
        at java.util.concurrent.CompletableFuture.uniHandle(CompletableFuture.java:822) ~[na:1.8.0_121]
        at java.util.concurrent.CompletableFuture$UniHandle.tryFire(CompletableFuture.java:797) ~[na:1.8.0_121]
        at java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:443) ~[na:1.8.0_121]
        at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289) ~[na:1.8.0_121]
        at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056) ~[na:1.8.0_121]
        at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692) ~[na:1.8.0_121]
        at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157) ~[na:1.8.0_121]

because of test:

    @Test(expected = TransportException.class)
    public void shouldFailEditOnBiddingFinished() throws Throwable {
TimMoore commented 7 years ago

Oh right... strange that it printed after the next test started. Are we missing an await?

ignasi35 commented 7 years ago

I don't think we're missing an await. I think it's a race condition when the logging. I'm not sure if the stack trace is printed by the server-side code or the client side code or the unit-test runner code though.

Long-story short, I don't think the exception is related to the contamination of tests reported in this issue's description originally.