spekframework / spek

A specification framework for Kotlin
Other
2.23k stars 181 forks source link

Support Kotlin/Native #555

Open charleskorn opened 5 years ago

charleskorn commented 5 years ago

I can see #327 added support for the multiplatform project model, but I can't see any references to Kotlin/Native.

Is this something you plan to support? If so, do you have an estimate for when?

And would you be open to a PR / PRs to add support for this?

raniejade commented 5 years ago

It will be supported eventually. PRs are definitely welcome!

charleskorn commented 5 years ago

Great to hear. Is there anything in particular you'd like to see a PR for? Based on a quick glance, it looks like the first step would be implementing the runtime and then some kind of test runner.

raniejade commented 5 years ago

Yep, more specifically:

Good place to start is the kotlin-test implementation for K/N. Is there a way to hook into it or something more native (think mocha and jasmine for JS).

charleskorn commented 5 years ago

Last time I looked at kotlin-test in K/N, there was a compile-time operation that scanned for tests and effectively generated a main() that then ran them (there's a bit more to it than that, but that's the general idea). The reason for this is that K/N doesn't (and won't) support reflection, so some other mechanism is required.

As an initial step, I'm thinking there could be something very simple that scans the source set for Spek classes and generates a main(), that can then be executed to execute and report on the tests. In the future this could be something more polished like a compiler plugin (for generating the main()) and a Gradle plugin (for hooking into Gradle's test infrastructure). How does that sound?

raniejade commented 5 years ago

Ideally we'd want something similar for JVM (so we can drop kotlin-reflect) and JS.

I'm thinking there could be something very simple that scans the source set for Spek classes and generates a main()

I think the only option we have (if we don't start with the compiler plugin) is to parse the test souces which doesn't look very simple to me.

raniejade commented 5 years ago

Did a quick spike on the compiler-plugin: https://github.com/raniejade/ct-discovery/blob/master/kotlin-plugin/src/main/kotlin/com/github/raniejade/ct/plugin.kt#L56

Basically I can see all classes being compiled and introspect annotations, super classes, etc. I just need a way to generate code.

charleskorn commented 5 years ago

That's really cool, it doesn't look too difficult to create a compiler plugin then.

https://github.com/JetBrains/kotlin-native/blob/master/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/TestProcessor.kt seems to be where most of the magic happens for kotlin-test.

I'm not going to have much time to work on this for a week or two - are you still interested in PRs if I get time after that, or will you look at it before then?

raniejade commented 5 years ago

PRs are always welcome! I'll post my progress here if I make any :joy:.

https://github.com/JetBrains/kotlin-native/blob/master/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/TestProcessor.kt seems to be where most of the magic happens for kotlin-test.

Ohh, wow it's generating IR. I was thinking if we could get away by just generating kotlin code and somehow include it in the build process.

charleskorn commented 5 years ago

I was hoping that too, but sadly not :)

OK, I think my rough plan in terms of steps towards this are:

  1. Basic implementation of the runtime and DSL for K/N, with users manually writing a main that passes a list of specs to a Spek-provided runner
  2. Gradle plugin to hook running the tests into the normal Gradle lifecycle
  3. Kotlin compiler plugin to generate the main automatically (and associated changes to the Gradle plugin to hook this up)

Does that sound sensible? I'm imagining each of these being a separate PR.

raniejade commented 5 years ago

Sounds like a plan 💯 .

with users manually writing a main that passes a list of specs to a Spek-provided runner

I'm imagining something like these:

class SomeTest: Spek({})
object AnotherTest: Spek({})

fun main() {
    val discoveryContext = DiscoveryContext.builder()
        .addClass<SomeTest>(SomeTest::new)
        .addClass<AnotherSpec> { AnotherTest }
        .build()

    val runtime = SpekRuntime(discoveryContext)
    val discoveryRequest = ...
    val discoveryResult = runtime.discover(discoveryRequest)
    val executionRequest = buildExecutionRequest(discoveryResult)
    runtime.execute(executionRequest)
}

DiscoveryContext can be also extended to include additional metadata, annotations for example.

charleskorn commented 5 years ago

At the moment, it looks like it's not possible to disable kotlin-test (the -tr compiler option is always added for test source sets). This isn't a showstopper, but it is a pain, so I've created https://github.com/JetBrains/kotlin-native/issues/2618 to track the issue.

charleskorn commented 5 years ago

I've had more time to work on this than expected, so I have an initial proof of concept for this working - please take a look at https://github.com/charleskorn/spek/commit/882b1c2b5d4727bf8f63ba86696e7009acdfe62f and let me know your thoughts.

Things that I will fix before creating a PR:

Things I think that can / should be improved, but might not have to be part of the initial PR:

raniejade commented 5 years ago

Wow! Awesome work @charleskorn, I'll take a look tonight!

raniejade commented 5 years ago

Looking good @charleskorn! I have a few comments in the commit. I think moving forward, lets break the changes into several small (more manageable) PRs.

  • the output from the test runner is very verbose and difficult to read
  • the test runner always exits with exit code 0, even if tests fail

I believe these points deserve a dedicated PR. I don't think we can hook up to gradle's testing infrasturcture to generate reports - so might need to do it ourselves.

the Travis CI build needs to be configured to build on all three OSes and to then publish these binaries (Kotlin/Native doesn't support cross compilation)

Separate PR, I'll create an issue.

the implementation of Path could probably be made common, with platform-specific implementations of Base64 encoding

Separate PR, I'll create an issue.

raniejade commented 5 years ago

Once this is addressed https://github.com/charleskorn/spek/commit/882b1c2b5d4727bf8f63ba86696e7009acdfe62f#r32176940, can you send in a PR?

charleskorn commented 5 years ago

Thanks for the feedback @raniejade!

I've already fixed the exit code issue and improved the format of the output somewhat (although there's plenty of room for improvement, and we probably need the option to select between different formats), so I'll include them in the PR once I address your comments.

charleskorn commented 5 years ago

To keep you updated: I've been working away at a compiler plugin - you can see my progress at https://github.com/charleskorn/spek/tree/compiler-plugin.

As soon as https://youtrack.jetbrains.com/issue/KT-29901 is resolved, I should be good to propose a PR.

raniejade commented 5 years ago

Awesome, thanks for the update! I've been busy as well setting up the CI builds. Other than linux, adding a build for osx should be easy now. For windows I might setup something in appveyor.

charleskorn commented 5 years ago

Nice! I think Travis supports Windows now, but not sure if it's in a production-ready state yet or not.

raniejade commented 5 years ago

I wasn't able to test it because it does not support language: java. I should be able to use language: shell and install the JDK by myself but that's a lot of work on a platform that may not be ready for use.

charleskorn commented 5 years ago

Right, makes sense.

raniejade commented 5 years ago

Windows support in travis looks promising. I've tried using language: shell and installed java myself on a new project that I'm working on: https://github.com/raniejade/termkolors/pull/1.

raniejade commented 3 years ago

Pushing this back since the kotlin compiler plugin api is not stable enough to my liking.