Skip to content

test: exclude timestamp from Node comparison in CreateNodeFromVertexActionTest#6321

Merged
westnordost merged 1 commit into
streetcomplete:masterfrom
RubenKelevra:fix_test_race_condition_in_create_node_from_vertex_action_create_updates
Jun 7, 2025
Merged

test: exclude timestamp from Node comparison in CreateNodeFromVertexActionTest#6321
westnordost merged 1 commit into
streetcomplete:masterfrom
RubenKelevra:fix_test_race_condition_in_create_node_from_vertex_action_create_updates

Conversation

@RubenKelevra

@RubenKelevra RubenKelevra commented Jun 7, 2025

Copy link
Copy Markdown
Contributor

Test compared Node objects including timestampEdited, causing failures when execution timing differed by milliseconds. Copy result timestamp to expected object to compare all fields except timing-dependent timestamp.

While running the test for #6318 the create updates in CreateNodeFromVertexActionTest failed due to a race condition, so I fixed it.

java.lang.AssertionError: expected:<MapDataChanges(creations=[], modifications=[Node(id=1, position=LatLon(latitude=0.0, longitude=0.0), tags={a=b}, version=1, timestampEdited=1749274176937)], deletions=[])> but was:<MapDataChanges(creations=[], modifications=[Node(id=1, position=LatLon(latitude=0.0, longitude=0.0), tags={a=b}, version=1, timestampEdited=1749274176936)], deletions=[])>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:120)
	at kotlin.test.junit.JUnitAsserter.assertEquals(JUnitSupport.kt:32)
	at kotlin.test.AssertionsKt__AssertionsKt.assertEquals(Assertions.kt:63)
	at kotlin.test.AssertionsKt.assertEquals(Unknown Source)
	at kotlin.test.AssertionsKt__AssertionsKt.assertEquals$default(Assertions.kt:62)
	at kotlin.test.AssertionsKt.assertEquals$default(Unknown Source)
	at de.westnordost.streetcomplete.data.osm.edits.create.CreateNodeFromVertexActionTest.create updates(CreateNodeFromVertexActionTest.kt:69)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
	at java.base/java.lang.reflect.Method.invoke(Method.java:565)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:112)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:40)
	at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:54)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:53)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
	at java.base/java.lang.reflect.Method.invoke(Method.java:565)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:92)
	at jdk.proxy1/jdk.proxy1.$Proxy4.processTestClass(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker$2.run(TestWorker.java:183)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:132)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:103)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:63)
	at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:122)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:72)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)

@RubenKelevra
test: exclude timestamp from Node comparison in CreateNodeFromVertexA…
4bcb184 
…ctionTest

Test compared Node objects including timestampEdited, causing failures
when execution timing differed by milliseconds. Copy result timestamp
to expected object to compare all fields except timing-dependent timestamp.
@RubenKelevra

RubenKelevra commented Jun 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@westnordost I've noticed that the date is used in some tests as well, so in theory if the date changes while the tests are running there are some more potential race conditions. But it's a can of worms ...

You want me to look into this, so the tests don't fail randomly by chance at midnight?

@westnordost

westnordost commented Jun 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Hmm... so the *Actions also update the element's timestampEdited to now, which is why the expected may be (1ms) off from the actual...

I bet this is not the only point where this could happen. (So, the PR currently doesn't fix the issue.)

No idea off the bat though how to fix it more holistically.

@RubenKelevra

RubenKelevra commented Jun 7, 2025

Copy link
Copy Markdown
Contributor Author

I have run the tests like 20 or 30 times now on my machine and this is the only test which failed for me on my machine because of this (twice). Have you seen random failures of other tests so far?

@westnordost

westnordost commented Jun 7, 2025

Copy link
Copy Markdown
Member

No

@westnordost

westnordost commented Jun 7, 2025

Copy link
Copy Markdown
Member

well then

@westnordost westnordost merged commit 989857c into streetcomplete:master Jun 7, 2025
@RubenKelevra RubenKelevra deleted the fix_test_race_condition_in_create_node_from_vertex_action_create_updates branch June 7, 2025 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@RubenKelevra @westnordost