arrow icon indicating copy to clipboard operation
arrow copied to clipboard

Increase test coverage Arrow Core

Open nomisRev opened this issue 2 years ago • 23 comments

This ticket tracks classes that are lacking in test coverage. All of this code has been stable for a long time, but it's a good way to get familiar with the Arrow codebase.

If you're looking to contribute to Arrow, and want a small piece of work writing some tests is a good way to get started. If you're not sure if a function, or code branch, is missing coverage in a certain file you can run ./gradlew koverHtmlReport and open the resulting report.

Or feel free to pick any of the items listed below and add missing tests for the .kt file.

Arrow Core

  • [x] currying.kt missing tests for almost all functions in progress by @lgtout
  • [x] [Either.kt]](https://github.com/arrow-kt/arrow/blob/main/arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Either.kt) only has 50% coverage in progress by @poseidon2060
  • [x] [Ior.kt]](https://github.com/arrow-kt/arrow/blob/main/arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Ior.kt) only has 70% coverage in progress by @gutiory
  • [ ] [Iterable.kt]](https://github.com/arrow-kt/arrow/blob/main/arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Iterable.kt) only has 50% coverage.
  • [ ] [MapKt.kt]](https://github.com/arrow-kt/arrow/blob/main/arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Iterable.kt) only has 16% coverage.
  • [x] [NonEmptyList.kt]](https://github.com/arrow-kt/arrow/blob/main/arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/NonEmptyList.kt) only has 50% coverage. @l2hyunwoo
  • [x] [Option.kt]](https://github.com/arrow-kt/arrow/blob/main/arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/NonEmptyList.kt) only has 75% coverage in progress by @swdevsm
  • [ ] [partials.kt]](https://github.com/arrow-kt/arrow/blob/main/arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/partials.kt) only has 2,4% coverage.
  • [ ] Missing tests for utilities TupleX.kt

nomisRev avatar Jan 19 '23 10:01 nomisRev

@raulraja @nomisRev Please assign this to me. Thanks!

lgtout avatar Jan 25 '23 05:01 lgtout

Hey, @nomisRev I'm going to try this issue out since I am not very well-versed with Kotlin Tests so this can be a good learning opportunity for me.😄 Can you brief me slightly on what to learn and how to approach this issue?

poseidon2060 avatar Feb 03 '23 01:02 poseidon2060

@nomisRev Multiple contributors working on the same issue. Should it be split? What do you propose? @poseidon2060 I'm working on currying.kt, so don't do that one. Thanks!
Posting on here which one you're working on might be a good idea, too.

lgtout avatar Feb 03 '23 02:02 lgtout

@nomisRev Multiple contributors working on the same issue. Should it be split? What do you propose? @poseidon2060 I'm working on currying.kt, so don't do that one. Thanks!
Posting on here which one you're working on might be a good idea, too.

Yes, I was planning to work on Either.kt anyways since I just worked on that file in another issue.

poseidon2060 avatar Feb 03 '23 03:02 poseidon2060

@poseidon2060 & @lgtout I added in the list that both are in progress by you, and checked the items. I would say to avoid having to create so many issues, we can just continue this approach.

If you want to work on something, or started working on something just tag me here and mention what you're working on and I'll update the original comment to reflect this. Thank you both for your contributions 🙏

nomisRev avatar Feb 03 '23 09:02 nomisRev

I'm sorry @nomisRev but I'll be a bit slow with this issue since I have my exams going on, I assure you that I'm slowly learning and working on it.

poseidon2060 avatar Feb 04 '23 06:02 poseidon2060

No worries at all @poseidon2060! Exams are more important, so take your time and focus on exams. The Either issue is reserved for you 😉 And best of luck! 🤞

nomisRev avatar Feb 04 '23 21:02 nomisRev

hey @nomisRev, I am available and can continue working on this issue, can you please guide me slightly on how to approach this issue? Thanks!😄

poseidon2060 avatar Feb 11 '23 08:02 poseidon2060

Hey @poseidon2060, Sorry for the late reply. In terms of guidance, we don't have any strict rules around tests but we favour property based tests. I'm not sure if you're familiar with those, but they test behavior without assuming anything about the generic code since Either for example is a structure that doesn't care about what values is nested inside E or A. You can find some more guidance here, https://kotest.io/docs/proptest/property-based-testing.html and inside of the EitherTest.kt file you will find many examples of what is currently already being tested.

To figure out what tests are missing I suggest running ./gradlew :arrow-core:koverHtmlReport and then checking the html page that is outputted by Gradle after the tasks finishes running. There you will find insights into which methods, or code branches, are missing test coverage.

Feel free to ask more questions here, or open a PR and I'd be happy to provide more guidance.

nomisRev avatar Feb 17 '23 10:02 nomisRev

As I've been working Ior API deprecation, I'm taking Ior 😎

gutiory avatar Feb 21 '23 11:02 gutiory

@nomisRev partials.kt needs the unreleased high-arity Kotest property testing methods, so I'll take it. Please update the checklist for me. Thanks!

lgtout avatar Feb 21 '23 23:02 lgtout

Hey @poseidon2060, Sorry for the late reply. In terms of guidance, we don't have any strict rules around tests but we favour property based tests. I'm not sure if you're familiar with those, but they test behavior without assuming anything about the generic code since Either for example is a structure that doesn't care about what values is nested inside E or A. You can find some more guidance here, https://kotest.io/docs/proptest/property-based-testing.html and inside of the EitherTest.kt file you will find many examples of what is currently already being tested.

To figure out what tests are missing I suggest running ./gradlew :arrow-core:koverHtmlReport and then checking the html page that is outputted by Gradle after the tasks finishes running. There you will find insights into which methods, or code branches, are missing test coverage.

Feel free to ask more questions here, or open a PR and I'd be happy to provide more guidance.

Hey @nomisRev, just wanted to update you on my progress with the issue I apologise I was not being very active with the assigned issue, I just had some work come up urgently with college I am finally free now and have started working on the issue, will give an update by tomorrow💪🫡

poseidon2060 avatar Feb 24 '23 16:02 poseidon2060

Hey @poseidon2060,

I apologise I was not being very active with the assigned issue, I just had some work come up urgently with college No worries at all! ☺️ School, and life, definitely comes first! Thank you for taking an interest and the time to look into and contribute to Arrow 🙏

nomisRev avatar Feb 24 '23 17:02 nomisRev

Hey @nomisRev,

I am not very experienced with testing but with help of link you provided and referring to already written tests, I came up with following test for flatMap function in Either Can you check if it's correct or not and point out the mistakes?

"flatMap should preserve Left value and transform the Right value" {
    checkAll(
      Arb.either(Arb.string(), Arb.int()),
      Arb.int().filter { it > 0 }
    ) { either, int ->
      val f = { b: Int -> Either.right(b * int) }
      when (either) {
        is Either.Left -> either.flatMap(f) == either
        is Either.Right -> either.flatMap(f) == (either.value * int).right()
      }
    }
  }

I may have made a lot of mistakes but I am trying to learn🫡

poseidon2060 avatar Feb 25 '23 20:02 poseidon2060

Ior tests are added in 818cab1 (#2928), inside the same API deprecatiion PR. New tests are added where missed, except for the deprecated methods.

gutiory avatar Feb 27 '23 12:02 gutiory

hi @nomisRev i have created a PR that contains a couple of tests for Iterable: https://github.com/arrow-kt/arrow/pull/2960

abendt avatar Mar 06 '23 22:03 abendt

Also i think there is a bug in the implementation of unalign. Its not exposed by the tests because Arb.ior only generates Ior.Both values. In effect some of branches in the logic are never executed. When the Arb is fixed (see PR) the test "align is the inverse of unalign" starts to fail.

abendt avatar Mar 06 '23 22:03 abendt

Hey @nomisRev Can you assign me on NonEmptyList.kt issue?

l2hyunwoo avatar Mar 11 '23 09:03 l2hyunwoo

Done @l2hyunwoo ☺️ Thank you for taking an interest to contributing. Feel free to ask any questions or doubts you have here or an a draft PR 😉

nomisRev avatar Mar 11 '23 10:03 nomisRev

Hi @nomisRev. I would also like to work on the Map tests. Please update the list

abendt avatar Mar 11 '23 15:03 abendt

@nomisRev Hello, is this task still valid? I would like to start working on it but I can see there is already an open PR for Iterable. Not sure about the last two items on the list.

daczczcz1 avatar Jul 24 '23 09:07 daczczcz1

Good day @nomisRev,

Could you assign me to the Tuples and Partials ones? The PR for the Tuples is #3143, with Partials up next :)

pimfm avatar Oct 13 '23 10:10 pimfm

I have some free time, assign me anything.

HOjjAT87 avatar Dec 01 '23 11:12 HOjjAT87