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

Please add test cases for the sake of helping users of the library #1037

Closed
frgomes opened this issue Nov 15, 2018 · 9 comments
Closed

Please add test cases for the sake of helping users of the library #1037

frgomes opened this issue Nov 15, 2018 · 9 comments
Milestone

Comments

@frgomes
Copy link

frgomes commented Nov 15, 2018

Test caseio.finch.iteratee.EnumerateEndpointSpec does not contain any useful test case from the users' perspective.

I'm trying to do something as simple as:

package com.example.helloworld

import cats.effect.IO
import com.twitter.finagle.Http
import com.twitter.util.Await
import io.iteratee.{Enumerator, Iteratee}
import io.finch._
import io.finch.circe._
import io.finch.iteratee._
import io.finch.catsEffect._

object Main extends App {

  def concat: Endpoint[IO, String] = post("concat" :: enumeratorBody[IO, String, Text.Plain]) {
    enum: Enumerator[IO, String] =>
      enum.into(Iteratee.fold[IO, String, String]("")(_ + _)).map(Ok)
  }

  Await.ready(
    Http.server
      .serve(":8081", (concat).toServiceAs[Text.Plain])
  )
}

with a deadly simple test case as

package com.example.helloworld

import org.scalatest.FunSuite

class MainTest extends FunSuite {
  test("helloWorld") {
    import io.finch._
    val body: String =
      """{"i":"aaaaa"}
        |{"i":"bbbbb"}
      """.stripMargin
    assert(
      Main.concat(
        Input
          .post("/concat")
          .withBody[Text.Plain](body)).awaitValueUnsafe() == Some("aaaaabbbbb"))
  }
}

and it does not work. So, it's clear that I'm doing something wrong... but what?

Looking for test cases, there's nothing which could provide any clue why things are going wrong.

So, please add test cases not only when things go wrong, but also showing what needs to be done in order to obtain positive results.

@frgomes
Copy link
Author

frgomes commented Nov 15, 2018

The main problem is that enumeratorBody[IO, Buf, Text.Plain] requires an implicit parameter which I do not have the slightest idea of what it could be and/or how it could be obtained.

I'm OK with code doing all sorts of magic. However, I'm not OK with the requirement of me having to be aware of someone else's magic in order to do something pretty simple as reading text from the wire.

The point is: in order to be useful, a library needs to provide a surface which is friendly for new users. If a new user is required to know the inner intrincate implementatation details of a library, the end result is a very steep learning curve and productivity zero for a number of weeks before anything useful can be finally written.

@sergeykolbasov
Copy link
Collaborator

sergeykolbasov commented Nov 15, 2018

Hey @frgomes

The thing about chunked bodies, that it should specifically mention that request is chunked, otherwise enumeratorBody wouldn't match.

Also, as I can see from the test, you're trying to decode JSON, but your endpoint uses quite "low-level" API enumeratorBody.
I suggest to switch to enumeratorJsonBody[IO, Foo] in case if you need JSON decoding.

Here you can find example of how to create proper chunked request using finagle's Writer and decode it with JSON:
https://github.com/finagle/finch/blob/master/iteratee/src/test/scala/io/finch/iteratee/EnumerateEndpointSpec.scala#L42

Here is an e2e example of streaming using iteratee:
https://github.com/finagle/finch/blob/master/examples/src/main/scala/io/finch/iteratee/Main.scala

Speaking of the examples and tests, I agree with you that there is a lack of proper up-to-date documentation that covers some library specifics, and we'd be glad to get a help here through documentation PRs.

@vkostyukov
Copy link
Collaborator

Related: looks like we also need to add support to streaming payloads in Input, to make this possible (easier). Something along the lines Input.withChunkedBody.

@frgomes
Copy link
Author

frgomes commented Nov 15, 2018

@vkostyukov : maybe entirely off-topic, but Finch does not help with the client side. I understand that we can resort to Finagle for that, which is OK for me... but the point is that we should have somehow a smoother learning path for writing and entire application using Finch, including test cases and e2e tests. At the moment, some effort and imagination is required in order to put it all together.

@frgomes
Copy link
Author

frgomes commented Nov 15, 2018

@sergeykolbasov : Yes, I'm aware of the source files you've mentioned. Despite of being aware of that, SBT dependencies are not mentioned, which is an issue at the moment for people transitioning to 0.25.0.

After I've collected the necessary dependencies, I've struggled a lot until I've finally got something working for the use case I have. Even tough the example on iteratees works, it's kind of "helloworld/foo/bar" which needs modification for being useful in real life; when you change that you face difficulties not covered by the example. So, I've suggested on #1018 that examples "closer to real life" are necessary.

@vkostyukov
Copy link
Collaborator

@frgomes

@joroKr21
Copy link
Collaborator

On an unrelated note it would be nice to have fs2 support (is there an issue?)

@vkostyukov
Copy link
Collaborator

On an unrelated note it would be nice to have fs2 support (is there an issue?)

It's here #1001

@vkostyukov vkostyukov modified the milestones: Finch 0.27, Finch 1.0 Dec 11, 2018
@vkostyukov
Copy link
Collaborator

I think we can close this. The upcoming 0.27 release has all the missing pieces outlined by this issue.

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

No branches or pull requests

4 participants