Minor improv. on `testing-plugin` & public CompilationResult #53

i-walker · 2019-11-01T11:46:16Z

In order for us to have HeavyTests in the ide, which verify that MetaSyntheticPackageFragementProvider does its job and all PsiElements result in with valid references to their typeclass, we need the outputdirectory in CompilationResult classesfiles. I renamed it for coherence. Concluding that the CompilationResult DT is public, but our interpreter is still internal and not discoverable for users.

I am open to Feedback, separating the compilation and the actual interpretation of that result seems also to be a good pattern.
Contrary, if we don't make CompilationResult public we can't establish HeavyTests in testing. As there is no way for us to generate the metadata in the editor.

rachelcarmena · 2019-11-01T15:39:58Z

compiler-plugin/src/test/kotlin/arrow/meta/plugins/comprehensions/ComprehensionsTest.kt

@@ -49,7 +49,7 @@ class ComprehensionsTest {
        listOf(addCompilerPlugins(compilerPlugin))
      },
      code = {
-        codeSnippet.source


I'd keep it because of: avoiding the use of primitives types, better types and it's planned to have several sources.

May you reiterate on what you mean with multiple sources?

I mean about different snippets.

that should be isomorphic to List<Source>. What am indicating here is that your not doing anything to your DataType which is domain-specific other than renaming String to Source

rachelcarmena · 2019-11-01T15:43:30Z

testing-plugin/src/main/kotlin/arrow/meta/plugin/testing/TestingLibraryWrapper.kt

  val actualStatus: CompilationStatus,
  val log: String,
  val actualGeneratedFilePath: Path,
-  val classesDirectory: File


There are different directories and we only use classes directory. I think outputDirectory could be confused with the outputDirectory of the testing library.

where is the outputDirectory of the testing library. I couldnt find it.

So, you're saying that classesDirectory is the same as outputDirectory?

They are different. That's the reason why I chose classesDirectory...

rachelcarmena · 2019-11-01T15:47:10Z

testing-plugin/src/main/kotlin/arrow/meta/plugin/testing/TestingLibraryWrapper.kt

@@ -10,11 +10,11 @@ import java.nio.file.Paths

 private const val DEFAULT_FILENAME = "Example.kt"

-internal data class CompilationResult(
+data class CompilationResult(


Instead of doing CompilationResult public, I'd think about the desired assertion. The purpose of this library is to provide a simple testing framework. If we publish the compilationResult, it seems this library doesn't make sense and we might use the kotlin compiler testing library directly.

I am not writing an assertion

I need the testing-library with the wrappers we added in testing-library. For the IDE it's a utility.

Please, could you give more details?

About what specifically?

About the use of the library in IDE...

rachelcarmena · 2019-11-01T15:53:08Z

@i-walker , despite of the comments, thanks for this pull request!! I think it could be useful to think about the code you'd like to be able to write. What's the desired assertion you would like to write? And then, we could think about how to change the library.

rachelcarmena · 2019-11-01T15:58:59Z

Another idea, @i-walker: use directly kotlin compiler testing library and then we'll think about a way to move your need to this library. I hope it's useful for you 🙏

i-walker · 2019-11-01T16:01:20Z

We can also focus on the more pressing issue, that other libraries won't be able to pull testing-library

{
  "errors" : [ {
    "status" : 409,
    "message" : "The repository 'oss-snapshot-local' rejected the resolution of an artifact 'oss-snapshot-local:com/github/arrow-kt/kotlin-compile-testing/1.3.0/kotlin-compile-testing-1.3.0.pom' due to conflict in the snapshot release handling policy."
  } ]
}

i-walker · 2019-11-01T16:06:12Z

This is a blocker for us because we already tried workarounds and pulling the underlying library directory and copy-pasting the code from testing-plugin doesn't seem to be an elegant solution.

rachelcarmena · 2019-11-01T17:44:43Z

We can also focus on the more pressing issue, that other libraries won't be able to pull testing-library

{
  "errors" : [ {
    "status" : 409,
    "message" : "The repository 'oss-snapshot-local' rejected the resolution of an artifact 'oss-snapshot-local:com/github/arrow-kt/kotlin-compile-testing/1.3.0/kotlin-compile-testing-1.3.0.pom' due to conflict in the snapshot release handling policy."
  } ]
}

Sorry, I don't understand this error. We are not deploying that library in oss.jfrog.org. We're using JitPack to get that dependency.

i-walker · 2019-11-01T18:09:05Z

Ok. How can we add testing-plugin to the dependencies in idea-plugin?

# Conflicts: # testing-plugin/src/main/kotlin/arrow/meta/plugin/testing/CompilationAssertions.kt # testing-plugin/src/main/kotlin/arrow/meta/plugin/testing/CompilerTestDSL.kt

…ng-plugin

rachelcarmena · 2019-11-01T18:38:51Z

Ok. How can we add testing-plugin to the dependencies in idea-plugin?

It's already included in compiler-plugin for compiling/executing tests:

testImplementation project(":testing-plugin")
testRuntimeOnly dependencies for compiler-plugin and the rest of the libraries which are necessary to compile snippets in tests
JitPack repository:

repositories {
    maven { url "https://jitpack.io" }
}

because testing-plugin is using the dependency of kotlin-compiler-testing which is only provided by JitPack right now.

cc: @jansorg

i-walker · 2019-11-01T19:16:09Z

Closing this

i-walker added 4 commits November 1, 2019 12:19

source is a typealias

ccd46d3

separates compilation from interpreter

cdd8476

flattens the ADT

f1eb333

rename classesfiles to outputdirectory

5a3263c

i-walker requested review from rachelcarmena, nomisRev and raulraja November 1, 2019 11:46

nomisRev and others added 2 commits November 1, 2019 13:49

Merge branch 'master' into is-testing-plugin

550afe1

Merge remote-tracking branch 'origin/master' into is-testing-plugin

faf8d22

rachelcarmena requested changes Nov 1, 2019

View reviewed changes

i-walker added 2 commits November 1, 2019 16:54

Merge branch 'master' into is-testing-plugin

1370680

Merge branch 'master' into is-testing-plugin

27924d3

jansorg mentioned this pull request Nov 1, 2019

[WIP] TypeClassResolutionTest for code snippets #57

Closed

Merge remote-tracking branch 'origin/master' into is-testing-plugin

34913a5

i-walker added 3 commits November 1, 2019 19:24

Merge remote-tracking branch 'origin/master' into is-testing-plugin

1c71baa

# Conflicts: # testing-plugin/src/main/kotlin/arrow/meta/plugin/testing/CompilationAssertions.kt # testing-plugin/src/main/kotlin/arrow/meta/plugin/testing/CompilerTestDSL.kt

Merge remote-tracking branch 'origin/master' into is-testing-plugin

5aa3ea0

Merge remote-tracking branch 'origin/is-testing-plugin' into is-testi…

2ee80a0

…ng-plugin

cleanUp

8b8d399

i-walker requested a review from rachelcarmena November 1, 2019 19:05

i-walker closed this Nov 1, 2019

i-walker deleted the is-testing-plugin branch November 1, 2019 19:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Minor improv. on `testing-plugin` & public CompilationResult #53

Minor improv. on `testing-plugin` & public CompilationResult #53

i-walker commented Nov 1, 2019

rachelcarmena Nov 1, 2019

i-walker Nov 1, 2019

rachelcarmena Nov 1, 2019

i-walker Nov 1, 2019

rachelcarmena Nov 1, 2019 •

edited

i-walker Nov 1, 2019

rachelcarmena Nov 1, 2019

i-walker Nov 1, 2019

rachelcarmena Nov 1, 2019

rachelcarmena Nov 1, 2019

i-walker Nov 1, 2019

i-walker Nov 1, 2019

rachelcarmena Nov 1, 2019

i-walker Nov 1, 2019

rachelcarmena Nov 1, 2019

rachelcarmena commented Nov 1, 2019 •

edited

rachelcarmena commented Nov 1, 2019

i-walker commented Nov 1, 2019

i-walker commented Nov 1, 2019

rachelcarmena commented Nov 1, 2019

i-walker commented Nov 1, 2019

rachelcarmena commented Nov 1, 2019

i-walker commented Nov 1, 2019

Minor improv. on testing-plugin & public CompilationResult #53

Minor improv. on testing-plugin & public CompilationResult #53

Conversation

i-walker commented Nov 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rachelcarmena Nov 1, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rachelcarmena commented Nov 1, 2019 • edited

rachelcarmena commented Nov 1, 2019

i-walker commented Nov 1, 2019

i-walker commented Nov 1, 2019

rachelcarmena commented Nov 1, 2019

i-walker commented Nov 1, 2019

rachelcarmena commented Nov 1, 2019

i-walker commented Nov 1, 2019

Minor improv. on `testing-plugin` & public CompilationResult #53

Minor improv. on `testing-plugin` & public CompilationResult #53

rachelcarmena Nov 1, 2019 •

edited

rachelcarmena commented Nov 1, 2019 •

edited