search

arrow-kt / arrow

Λrrow - Functional companion to Kotlin's Standard Library
http://arrow-kt.io
Other
6.16k stars 444 forks source link

Increase test coverage Arrow Core #2894

Open nomisRev opened 1 year ago

nomisRev commented 1 year ago

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

lgtout commented 1 year ago

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

poseidon2060 commented 1 year ago

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?

lgtout commented 1 year ago

@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.

poseidon2060 commented 1 year ago

@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.

nomisRev commented 1 year ago

@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 🙏

poseidon2060 commented 1 year ago

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.

nomisRev commented 1 year ago

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! 🤞

poseidon2060 commented 1 year ago

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!😄

nomisRev commented 1 year ago

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.

gutiory commented 1 year ago

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

lgtout commented 1 year ago

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

poseidon2060 commented 1 year ago

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💪🫡

nomisRev commented 1 year ago

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 🙏

poseidon2060 commented 1 year ago

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🫡

gutiory commented 1 year ago

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

abendt commented 1 year ago

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

abendt commented 1 year ago

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.

l2hyunwoo commented 1 year ago

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

nomisRev commented 1 year ago

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 😉

abendt commented 1 year ago

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

daczczcz1 commented 1 year ago

@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.

pimfm commented 11 months ago

Good day @nomisRev,

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

HOjjAT87 commented 10 months ago

I have some free time, assign me anything.