Skip to content
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

Use kotlin.test instead of JUnit #5220

Merged

Conversation

riQQ
Copy link
Collaborator

@riQQ riQQ commented Aug 28, 2023

Almost everything works, but there is one new test failure in androidTest NoteQuestsHiddenDaoTest.getNewerThan on a Pixel 4 emulator that I don't know what it's caused by:

java.lang.NoSuchMethodError: No static method boxLong(J)Ljava/lang/Long; in class Lkotlin/coroutines/jvm/internal/Boxing; or its super classes (declaration of 'kotlin.coroutines.jvm.internal.Boxing' appears in /data/app/~~Cmbt4P5atgm8GMNc69RGiA==/de.westnordost.streetcomplete.debug-n9y365mjtzOc6KdZczYHAg==/base.apk)
at de.westnordost.streetcomplete.data.osmnotes.notequests.NoteQuestsHiddenDaoTest$getNewerThan$1.invokeSuspend(NoteQuestsHiddenDaoTest.kt:49)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTaskKt.resume(DispatchedTask.kt:254)
at kotlinx.coroutines.DispatchedTaskKt.dispatch(DispatchedTask.kt:166)
at kotlinx.coroutines.CancellableContinuationImpl.dispatchResume(CancellableContinuationImpl.kt:474)
at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl(CancellableContinuationImpl.kt:508)
at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl$default(CancellableContinuationImpl.kt:497)
at kotlinx.coroutines.CancellableContinuationImpl.resumeUndispatched(CancellableContinuationImpl.kt:595)
at kotlinx.coroutines.EventLoopImplBase$DelayedResumeTask.run(EventLoop.common.kt:493)
at kotlinx.coroutines.EventLoopImplBase.processNextEvent(EventLoop.common.kt:280)
at kotlinx.coroutines.BlockingCoroutine.joinBlocking(Builders.kt:85)
at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Builders.kt:59)
at kotlinx.coroutines.BuildersKt.runBlocking(Unknown Source:1)
at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking$default(Builders.kt:38)
at kotlinx.coroutines.BuildersKt.runBlocking$default(Unknown Source:1)
at de.westnordost.streetcomplete.data.osmnotes.notequests.NoteQuestsHiddenDaoTest.getNewerThan(NoteQuestsHiddenDaoTest.kt:43)

The error occurs in line 49 assertEquals(2L, result.noteId).

@Test fun getNewerThan() = runBlocking {
dao.add(1L)
delay(200)
val time = nowAsEpochMilliseconds()
dao.add(2L)
val result = dao.getNewerThan(time - 100).single()
assertEquals(2L, result.noteId)
}

And another question regarding code style of imports and consistency.
Some files use star import à la import org.junit.Assert.*. Should this be kept, all files changed to star import or all files changed to explicit imports?

@westnordost
Copy link
Member

Oh, cool!

For me, * imports for the test framework stuff is fine.

@riQQ
Copy link
Collaborator Author

riQQ commented Aug 28, 2023

So change all the test imports to import kotlin.test.*?

What do we do about the new test failure? Any idea? I haven't found anything helpful after searching the web.

@FloEdelmann
Copy link
Member

So change all the test imports to import kotlin.test.*?

Some other contributors (including me) prefer non-star imports, because you can more easily see where an import is coming from, also without an IDE. So I'd say let's leave it as it is.

@westnordost
Copy link
Member

It's up to you, I'm fine with it either way.

Especially for tests though, IMO it is not necessary to know where the particular assert is coming from, as it is used pretty much in every test function.

Regarding the test failure - it concerns the androidTests, no? I understand that you only changed the tests. Anyway, I guess it is some kind of auto-boxing failure. I may have a look at that some time later. To be honest, I kind of ... don't like that all the database tests are in androidTest as if it was dependent on Android. Well, it kind of is a little bit, since the SQLite API is built-in into Android without an extra dependency. But with relatively little effort, I think all those database tests could be migrated into the normal test code.

@riQQ
Copy link
Collaborator Author

riQQ commented Aug 28, 2023

Regarding the test failure - it concerns the androidTests, no? I understand that you only changed the tests.

Yes and yes

Anyway, I guess it is some kind of auto-boxing failure.

Any hint on where to read up on this? A quick search didn't return anything useful.

@westnordost
Copy link
Member

westnordost commented Aug 28, 2023 via email

@westnordost westnordost merged commit 533b847 into streetcomplete:master Aug 28, 2023
@riQQ riQQ deleted the test/use-kotlin-test-instead-of-junit branch August 28, 2023 19:42
@matkoniecz matkoniecz mentioned this pull request Sep 1, 2023
@westnordost westnordost mentioned this pull request Dec 20, 2023
@FloEdelmann

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Released
Development

Successfully merging this pull request may close these issues.

3 participants