-
Notifications
You must be signed in to change notification settings - Fork 8
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
Fix CI pipeline #13
Fix CI pipeline #13
Changes from 21 commits
ed2f6a9
1d8fb1b
6cd0eb1
f6eccd9
2b4f6aa
1a0ff99
a9f2460
62c64c5
e921362
b1cd2ec
8b8c045
b24746d
d8ceb05
936d593
0c24b5d
0865492
d1ac0c4
12cca52
50ff96a
5fde5d4
6721c4d
5374b60
98dd492
482bd81
705c833
901504e
13400cb
3fe2d7b
6b0fd1c
bcc7d34
03d98b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,9 @@ | ||
package org.junit.runner | ||
|
||
import org.typelevel.scalaccompat.annotation.unused | ||
|
||
/** | ||
* Stub used for cross-compilation | ||
*/ | ||
class RunWith[T](cls: Class[T]) extends scala.annotation.StaticAnnotation | ||
class RunWith[T](@unused cls: Class[T]) | ||
extends scala.annotation.StaticAnnotation |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,8 @@ | ||
package weaver | ||
|
||
import org.typelevel.scalaccompat.annotation.unused | ||
private[weaver] object PlatformCompat { | ||
val platform: Platform = Platform.JS | ||
|
||
def getClassLoader(clazz: java.lang.Class[_]): ClassLoader = | ||
def getClassLoader(@unused clazz: java.lang.Class[_]): ClassLoader = | ||
new ClassLoader() {} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
package org.junit.runner | ||
import org.typelevel.scalaccompat.annotation.unused | ||
|
||
/** | ||
* Stub used for cross-compilation | ||
*/ | ||
class RunWith[T](cls: Class[T]) extends scala.annotation.StaticAnnotation | ||
class RunWith[T](@unused cls: Class[T]) | ||
extends scala.annotation.StaticAnnotation |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,9 @@ | ||
package weaver | ||
import org.typelevel.scalaccompat.annotation.unused | ||
|
||
private[weaver] object PlatformCompat { | ||
val platform: Platform = Platform.Native | ||
|
||
def getClassLoader(clazz: java.lang.Class[_]): ClassLoader = | ||
def getClassLoader(@unused clazz: java.lang.Class[_]): ClassLoader = | ||
new ClassLoader() {} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,6 @@ import cats.effect.{ Async, Resource } | |
import cats.syntax.all._ | ||
|
||
import fs2.Stream | ||
import org.junit.runner.RunWith | ||
import org.portablescala.reflect.annotation.EnableReflectiveInstantiation | ||
|
||
// Just a non-parameterized marker trait to help SBT's test detection logic. | ||
|
@@ -60,7 +59,6 @@ object EffectSuite { | |
|
||
} | ||
|
||
@RunWith(classOf[weaver.junit.WeaverRunner]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mmm this annotation has a pretty significant importance (despite the fact that there's no test associated to it): it's what allows IntelliJ to run test suites by clicking on them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought it was important, but wanted to get to a green build before fixing it. Our doc generation fails in Scala 3:
I haven't dug deep into it yet, but it seems like a bug in dottydoc or the tasty reader. Possibly, it can't read the the tasty generated by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's probably because the dependency is tagged as Optional in this module. We could make the dependency required in the scope of the task that requires it maybe ? Not gonna lie, my SBT skills are not good enough to know how to do that from the top of my head There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That seems to be the problem.
The classpath rules are the same for Scala 2.13 and 3, but dottydoc uses the tasty reader, which seems to need all jars to be present. We can resolve this by adding the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's great, thank you ! |
||
abstract class RunnableSuite[F[_]] extends EffectSuite[F] { | ||
implicit protected def effectCompat: UnsafeRun[EffectType] | ||
private[weaver] def getEffectCompat: UnsafeRun[EffectType] = effectCompat | ||
|
@@ -131,13 +129,13 @@ abstract class MutableFSuite[F[_]] extends RunnableSuite[F] { | |
} | ||
|
||
def pureTest(name: TestName)(run : => Expectations) : Unit = registerTest(name)(_ => Test(name.name, effectCompat.effect.delay(run))) | ||
def loggedTest(name: TestName)(run: Log[F] => F[Expectations]) : Unit = registerTest(name)(_ => Test(name.name, log => run(log))) | ||
def loggedTest(name: TestName)(run: Log[F] => F[Expectations]) : Unit = registerTest(name)(_ => Test[F](name.name, log => run(log))) | ||
def test(name: TestName) : PartiallyAppliedTest = new PartiallyAppliedTest(name) | ||
|
||
class PartiallyAppliedTest(name : TestName) { | ||
def apply(run: => F[Expectations]) : Unit = registerTest(name)(_ => Test(name.name, run)) | ||
def apply(run : Res => F[Expectations]) : Unit = registerTest(name)(res => Test(name.name, run(res))) | ||
def apply(run : (Res, Log[F]) => F[Expectations]) : Unit = registerTest(name)(res => Test(name.name, log => run(res, log))) | ||
def apply(run : (Res, Log[F]) => F[Expectations]) : Unit = registerTest(name)(res => Test[F](name.name, log => run(res, log))) | ||
|
||
// this alias helps using pattern matching on `Res` | ||
def usingRes(run : Res => F[Expectations]) : Unit = apply(run) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -132,7 +132,7 @@ object MetaJVM { | |
} | ||
} | ||
|
||
abstract class LazyAccessSequential(global: GlobalRead, index: Int) | ||
abstract class LazyAccessSequential(global: GlobalRead) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This |
||
extends IOSuite { | ||
type Res = LazyState | ||
def sharedResource: Resource[IO, Res] = { | ||
|
@@ -157,10 +157,10 @@ object MetaJVM { | |
|
||
// Using sleeps to force sequential runs of suites | ||
class LazyAccessSequential0(global: GlobalRead) | ||
extends LazyAccessSequential(global, 0) | ||
extends LazyAccessSequential(global) | ||
class LazyAccessSequential1(global: GlobalRead) | ||
extends LazyAccessSequential(global, 1) | ||
extends LazyAccessSequential(global) | ||
class LazyAccessSequential2(global: GlobalRead) | ||
extends LazyAccessSequential(global, 2) | ||
extends LazyAccessSequential(global) | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
package weaver | ||
package framework | ||
import org.typelevel.scalaccompat.annotation.unused | ||
|
||
import java.io.PrintStream | ||
import java.util.concurrent.ConcurrentLinkedQueue | ||
|
@@ -37,7 +38,7 @@ trait RunnerCompat[F[_]] { self: sbt.testing.Runner => | |
} | ||
|
||
// Required on js | ||
def receiveMessage(msg: String): Option[String] = None | ||
def receiveMessage(@unused msg: String): Option[String] = None | ||
|
||
// Flag meant to be raised if build-tool call `done` | ||
protected val isDone: AtomicBoolean = new AtomicBoolean(false) | ||
|
@@ -57,9 +58,9 @@ trait RunnerCompat[F[_]] { self: sbt.testing.Runner => | |
val stillRunning = new AtomicInteger(0) | ||
val waitForResourcesShutdown = new java.util.concurrent.Semaphore(0) | ||
|
||
val tasksAndSuites = taskDefs.toList.map { taskDef => | ||
taskDef -> suiteLoader(taskDef) | ||
}.collect { case (taskDef, Some(suite)) => (taskDef, suite) } | ||
val tasksAndSuites = (taskDefs.toList.mapFilter { taskDef => | ||
suiteLoader(taskDef).map(loader => (taskDef, loader)) | ||
}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous code flags up an unreachable case warning in Scala 3, although I'm not sure why:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Til about mapFilter ! |
||
|
||
def makeTasks( | ||
taskDef: TaskDef, | ||
|
@@ -240,7 +241,7 @@ trait RunnerCompat[F[_]] { self: sbt.testing.Runner => | |
.productR(broker.send(TestFinished(outcome))), | ||
finalizer) | ||
} | ||
)).handleErrorWith { case scala.util.control.NonFatal(_) => | ||
)).recoverWith { case scala.util.control.NonFatal(_) => | ||
effect.unit // avoid non-fatal errors propagating up | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,7 +38,7 @@ abstract class DogFood[F[_]]( | |
for { | ||
eventHandler <- effect.delay(new MemoryEventHandler()) | ||
logger <- effect.delay(new MemoryLogger()) | ||
_ <- getTasks(suites, logger).use { case (runner, tasks) => | ||
_ <- getTasks(suites).use { case (runner, tasks) => | ||
runTasks(runner, eventHandler, logger, maxParallelism)(tasks.toList) | ||
} | ||
_ <- patience.fold(effect.unit)(framework.unsafeRun.sleep) | ||
|
@@ -71,8 +71,8 @@ abstract class DogFood[F[_]]( | |
} | ||
|
||
private def getTasks( | ||
suites: Seq[Fingerprinted], | ||
logger: Logger): Resource[F, (WeaverRunner[F], Array[sbt.testing.Task])] = { | ||
suites: Seq[Fingerprinted] | ||
): Resource[F, (WeaverRunner[F], Array[sbt.testing.Task])] = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This |
||
val acquire = Sync[F].delay { | ||
val cl = PlatformCompat.getClassLoader(this.getClass()) | ||
framework.weaverRunner(Array(), Array(), cl, None) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,6 @@ import scala.reflect.ClassTag | |
import cats.effect.Sync | ||
|
||
import weaver.internals.Reflection._ | ||
import weaver.{ EffectSuite, GlobalResourceF } | ||
|
||
import sbt.testing.{ Fingerprint, SubclassFingerprint, TaskDef } | ||
|
||
|
@@ -59,6 +58,7 @@ abstract class WeaverFingerprints[F[_]](implicit F: Sync[F]) { | |
loadModule(taskDef.fullyQualifiedName(), classLoader) | ||
val init = cast(module)(GlobalResourcesInitClass) | ||
Some(GlobalResourcesRef(init)) | ||
case _ => None | ||
} | ||
|
||
} | ||
|
@@ -76,7 +76,7 @@ abstract class WeaverFingerprints[F[_]](implicit F: Sync[F]) { | |
/** | ||
* A fingerprint that searches only for classes extending | ||
* [[weaver.EffectSuite]]. that have a constructor that takes a single | ||
* [[weaver.GlobalResources.Read]] parameter. | ||
* `weaver.GlobalResources.Read` parameter. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. scaladoc can't find this symbol:
Should this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so, indeed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I'm concerned, this is the last outstanding item for this PR to be merged 🎉 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hah, good call, always mix them for one another |
||
*/ | ||
object ResourceSharingSuiteFingerprint extends WeaverFingerprint { | ||
def isModule() = false | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We get the following error in 2.12, resolved by adding these type parameters: