feat(scala-sdk): Integration test template by raboof · Pull Request #565 · lightbend/kalix-jvm-sdk

raboof · 2021-10-05T12:07:43Z

Part of #372

octonato

LGTM

with small suggestions

octonato · 2021-10-05T12:11:42Z

+      ValueEntityTestKitGenerator.integrationTest(main, entity, service).content,
+      """package com.example.shoppingcart.api
+
+import com.akkaserverless.scalasdk.testkit.AkkaServerlessTestkit


Are we moving out of using the |? I have see that at least one other suite were not using it.

I don't mind. Maybe be even easier to read. Soon I will be write codegen for the replicated entity. I will use that new style then.

Oh, this was not particularly intentional, I'm fine with either

Maybe be easier if we remove the |.

since we have mostly used | I think we should continue with that, and at least to me it reads a bit weird to suddenly have code to the very left

octonato · 2021-10-05T12:13:45Z

+ * testkit before testing the service with gRPC or HTTP clients. Call {@link #stop} after tests are complete.
+ */
+class AkkaServerlessTestkit(impl: JTestKit) {
+  def start() = impl.start()


nitpicking:

We have been using impl, but I think we should call it delegate. Both are implementations, but one is an adapter that delegates all calls to the other impl.

Co-authored-by: Renato Cavalcanti <[email protected]>

…nSpec

patriknw · 2021-10-05T15:41:20Z

+
+  def stop() = delegate.stop()
+}
+object AkkaServerlessTestkit {


can we have this at the top, since that is the starting point for using it?

patriknw · 2021-10-05T15:41:38Z

+ * <p>Create an AkkaServerlessTestkit with an {@link AkkaServerless} service descriptor, and then {@link #start} the
+ * testkit before testing the service with gRPC or HTTP clients. Call {@link #stop} after tests are complete.
+ */
+class AkkaServerlessTestkit(delegate: JTestKit) {


make the constructor private

patriknw · 2021-10-05T15:43:03Z

+
+  val testkit = AkkaServerlessTestkit(Main.createAkkaServerless())
+  testkit.start()
+  implicit val system = testkit.system


system: ActorSystem to avoid warnings in Intellij

is that needed for running streaming calls, or something else? wonder if we should not include it since it exposes a bit too much low level Akka? btw, if it's for streams we should have it as Materializer since that is what we have in the contexts

It's used for creating the Akka gRPC client

ah, we could have a grpcClient utility method similar to what we provide in the context. That would become more unified. Can be a separate ticket.

I created issue #568 for this idea

patriknw · 2021-10-05T15:45:07Z

+
+  def system = delegate.getActorSystem()
+
+  def stop() = delegate.stop()


explicit return type on all things here

patriknw · 2021-10-05T15:47:41Z

+    val entityClassName = service.fqn.name
+
+    File(
+      service.fqn.fileBasename + "IntegrationSpec.scala",


if we have these in test scope I wonder if there is a way to run all unit tests without these?

Thinking about testOnly **Spec but that would include IntegrationSpec too. Shall we name the others UnitSpec so that testOnly **UnitSpec would work?

*UnitSpec could work, though it's a bit unfortunate when ideally you'd want to have many unit tests and few integration tests. The only alternative I can think of is using ScalaTest tagging, though that's a bit obscure.

Created issue #571 for this thought

patriknw · 2021-10-05T15:50:43Z

+      ValueEntityTestKitGenerator.integrationTest(main, entity, service).content,
+      """package com.example.shoppingcart.api
+
+import com.akkaserverless.scalasdk.testkit.AkkaServerlessTestkit


since we have mostly used | I think we should continue with that, and at least to me it reads a bit weird to suddenly have code to the very left

johanandren · 2021-10-05T18:05:17Z

+ * <p>Create an AkkaServerlessTestkit with an {@link AkkaServerless} service descriptor, and then {@link #start} the
+ * testkit before testing the service with gRPC or HTTP clients. Call {@link #stop} after tests are complete.
+ */
+object AkkaServerlessTestkit {


Different casing than all other test kits

Suggested change

object AkkaServerlessTestkit {

object AkkaServerlessTestKit {

well, the javadsl AkkaServerlessTestkit is also lowercase-k. I'll also update that one then ;)

Especially important for codegenCore

patriknw

LGTM

patriknw · 2021-10-06T11:58:26Z

+    val entityClassName = service.fqn.name
+
+    File(
+      service.fqn.fileBasename + "IntegrationSpec.scala",


Created issue #571 for this thought

raboof added 3 commits October 5, 2021 11:56

wip

8942fc5

Add codegen unit test

5f53aae

Attaching it to the service, not the entity

244123a

octonato approved these changes Oct 5, 2021

View reviewed changes

raboof and others added 7 commits October 5, 2021 14:22

Add 'unmanagedComment' to integration test

6661791

Use a package in the unittest

f6dc0b9

Publish the java testkit as well

f6c9bc8

impl -> delegate

1175894

add space

8beb5ba

Co-authored-by: Renato Cavalcanti <[email protected]>

Update scripted test expectation

6655a98

Merge remote-tracking branch 'origin/integrationSpec' into integratio…

9e241c2

…nSpec

patriknw reviewed Oct 5, 2021

View reviewed changes

raboof added 5 commits October 5, 2021 19:08

object to top

1e5dc9f

Explicit return values

dce78fc

indent

9571329

Explicit type for implicit

133dcd6

scalafmt

2cf37c1

johanandren reviewed Oct 5, 2021

View reviewed changes

patriknw mentioned this pull request Oct 6, 2021

grpcClient utility method in integration tests #568

Closed

raboof added 4 commits October 6, 2021 10:24

'TestKit' with capital K

9168f10

Target jdk8

b2eaee1

Especially important for codegenCore

Add docs

a071073

scalafmt

364a3c9

patriknw mentioned this pull request Oct 6, 2021

Convenient way to only run unit tests #571

Closed

patriknw approved these changes Oct 6, 2021

View reviewed changes

More capital 'K'

abcd7bf

raboof force-pushed the integrationSpec branch from 5690e94 to abcd7bf Compare October 6, 2021 12:33

Actually add CounterServiceIntegrationSpec.scala

a66eb26

raboof mentioned this pull request Oct 6, 2021

Scala SDK #372

Closed

21 tasks

johanandren approved these changes Oct 6, 2021

View reviewed changes

raboof merged commit 1bdbe61 into main Oct 6, 2021

raboof deleted the integrationSpec branch October 6, 2021 14:10


		def system = delegate.getActorSystem()

		def stop() = delegate.stop()

	object AkkaServerlessTestkit {
	object AkkaServerlessTestKit {

Conversation

raboof commented Oct 5, 2021

Uh oh!

octonato left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raboof Oct 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patriknw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

raboof Oct 5, 2021 •

edited

Loading