Fix CI pipeline #13

zainab-ali · 2024-03-08T15:53:21Z

This ~~almost~~ fixes the CI pipeline.

Updates the scalafmt config.
Adds type parameters to make Scala 2.12 happy.
Removes unused imports.
Adds scalac-compat-annotation and a load of unused annotations.
Adds junit as a scaladoc dependency for coreCats.

I'm hesitant to eagerly remove unused parameters as I don't have the context as to why they're there. If you'd like any removed, drop a comment next to them.

~~The only outstanding issue is doc generation and the RunWith annotation. This is likely a scaladoc / tasty bug and needs a bit more investigation.~~

Baccata · 2024-03-12T13:47:13Z

modules/core/shared/src/main/scala/weaver/suites.scala

@@ -60,7 +59,6 @@ object EffectSuite {

 }

-@RunWith(classOf[weaver.junit.WeaverRunner])


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.

I thought it was important, but wanted to get to a green build before fixing it.

Our doc generation fails in Scala 3:

sbt:cats-core> doc [error] -- Error: modules/core/shared/src/main/scala/weaver/suites.scala:63:0 ---------- [error] 63 |@RunWith(classOf[weaver.junit.WeaverRunner]) [error] |^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [error] |undefined: new org.junit.runner.RunWith # -1: TermRef(TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class junit)),object runner),RunWith),<init>) at readTasty [error] one error found

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 suites.scala.

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

That seems to be the problem.

junit is an Optional dependency of core.

coreJVM / doc has the junit jar on the classpath.

coreCats depends on core. However coreCatsJVM / doc doesn't have the junit jar.

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 junit dependency with a ScalaDocTool configuration.

that's great, thank you !

zainab-ali · 2024-03-12T16:17:28Z

modules/core/shared/src/main/scala/weaver/Test.scala

@@ -40,6 +40,6 @@ object Test {

  def apply[F[_]](name: String, f: F[Expectations])(
      implicit F: EffectCompat[F]
-  ): F[TestOutcome] = apply(name, (_: Log[F]) => f)
+  ): F[TestOutcome] = apply[F](name, (_: Log[F]) => f)


We get the following error in 2.12, resolved by adding these type parameters:

[warn] compiler bug: Unexpected code path: testing two type variables for subtype relation: [warn] ?F <:< ?F [warn] Please report bug at https://github.com/scala/bug/issues

zainab-ali · 2024-03-12T16:20:30Z

modules/framework/jvm/src/main/scala/RunnerCompat.scala

-    }.collect { case (taskDef, Some(suite)) => (taskDef, suite) }
+    val tasksAndSuites = (taskDefs.toList.mapFilter { taskDef =>
+      suiteLoader(taskDef).map(loader => (taskDef, loader))
+    })


The previous code flags up an unreachable case warning in Scala 3, although I'm not sure why:

[warn] 63 | }.collect { case (taskDef, Some(suite)) => (taskDef, suite) } [warn] | ^^^^^^^^^^^^^^^^^^^^^^ [warn] | Unreachable case

Til about mapFilter !

zainab-ali · 2024-03-12T16:21:28Z

modules/framework/shared/src/main/scala/weaver/framework/DogFood.scala

-      suites: Seq[Fingerprinted],
-      logger: Logger): Resource[F, (WeaverRunner[F], Array[sbt.testing.Task])] = {
+      suites: Seq[Fingerprinted]
+  ): Resource[F, (WeaverRunner[F], Array[sbt.testing.Task])] = {


This logger looked safe enough to remove.

zainab-ali · 2024-03-12T16:25:52Z

modules/framework/shared/src/main/scala/weaver/framework/Fingerprints.scala

@@ -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.


scaladoc can't find this symbol:

[warn] 81 | object ResourceSharingSuiteFingerprint extends WeaverFingerprint { [warn] | ^ [warn] | No DRI found for query: weaver.GlobalResources.Read

Should this be weaver.GlobalRead?

I think so, indeed

As far as I'm concerned, this is the last outstanding item for this PR to be merged 🎉

GlobalRead came from the cats module, so I went for weaver.GlobalResourceF.Read instead.

hah, good call, always mix them for one another

zainab-ali · 2024-03-12T16:39:11Z

modules/core/shared/src/main/scala/weaver/suites.scala

@@ -60,7 +59,6 @@ object EffectSuite {

 }

-@RunWith(classOf[weaver.junit.WeaverRunner])


I thought it was important, but wanted to get to a green build before fixing it.

Our doc generation fails in Scala 3:

sbt:cats-core> doc [error] -- Error: modules/core/shared/src/main/scala/weaver/suites.scala:63:0 ---------- [error] 63 |@RunWith(classOf[weaver.junit.WeaverRunner]) [error] |^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [error] |undefined: new org.junit.runner.RunWith # -1: TermRef(TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class junit)),object runner),RunWith),<init>) at readTasty [error] one error found

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 suites.scala.

zainab-ali · 2024-03-13T14:00:52Z

modules/framework-cats/jvm/src/test/scala/MetaJVM.scala

@@ -132,7 +132,7 @@ object MetaJVM {
    }
  }

-  abstract class LazyAccessSequential(global: GlobalRead, index: Int)
+  abstract class LazyAccessSequential(global: GlobalRead)


This index stopped being used as of this commit to improve the sequential access tests. We can safely get rid of it.

zainab-ali · 2024-03-13T14:04:47Z

We're all green! I'm happy with merging.

Set scala-3 dialect to Scala 3.

ed2f6a9

zainab-ali marked this pull request as draft March 8, 2024 15:53

zainab-ali added 15 commits March 8, 2024 16:03

Remove unused imports.

1d8fb1b

Remove unused dummy parameter.

6cd0eb1

Remove unused code.

f6eccd9

Refactor code to remove unreachable case warning.

2b4f6aa

Remove unused imports for Scala 3 Test JVM.

1a0ff99

Remove unused integer from LazyAccessSequential.

a9f2460

Use msg instead of cat for Scala 2/3 warnings.

62c64c5

Use recoverWith with partial function.

e921362

Use runner.

b1cd2ec

Add a None case for comprehensive matching.

8b8c045

Remove unused implicit numeric widening annotation.

b24746d

Remove unused private var.

d8ceb05

Mark cls as unused (broken for 2.12).

936d593

Reformat.

0c24b5d

Mark RunWith as unused.

0865492

zainab-ali changed the title ~~Set scala-3 dialect to Scala 3.~~ Fix CI pipeline Mar 8, 2024

zainab-ali added 5 commits March 11, 2024 14:35

Add newline to scalafmt.conf.

d1ac0c4

Use scalacCompatAnnotation.

12cca52

Add type parameters for Scala 2.12 compiler subtype warning.

50ff96a

Fix doc for 2.13

5fde5d4

Experiment: remove RunWith annotation.

6721c4d

Baccata reviewed Mar 12, 2024

View reviewed changes

zainab-ali added 6 commits March 12, 2024 16:00

Add unused Reader.pt, WeaverRunner.dummy, LazyAccessSequential.index

5374b60

Add WeaverRunner constructor back.

98dd492

Use nowarn2 annotation.

482bd81

Mark runner in DogFoodCompat as unused.

705c833

Use tupleLeft.

901504e

Add RunWith annotation (and fail CI pipeline).

13400cb

zainab-ali commented Mar 12, 2024

View reviewed changes

zainab-ali marked this pull request as ready for review March 12, 2024 16:48

zainab-ali added 4 commits March 13, 2024 13:20

Add optional JUnit dependency to cats-core.

3fe2d7b

Use ScalaDocTool configuration.

6b0fd1c

Remove unused index from LazyAccessSequential test.

bcc7d34

Use GlobalResourceF.Read in scaladoc.

03d98b2

zainab-ali commented Mar 13, 2024

View reviewed changes

Baccata approved these changes Mar 13, 2024

View reviewed changes

Baccata merged commit 1e57875 into typelevel:sbt-typelevel Mar 13, 2024
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix CI pipeline #13

Fix CI pipeline #13

zainab-ali commented Mar 8, 2024 •

edited

Loading

Baccata Mar 12, 2024

zainab-ali Mar 12, 2024

Baccata Mar 12, 2024

zainab-ali Mar 13, 2024 •

edited

Loading

Baccata Mar 13, 2024

zainab-ali Mar 12, 2024

zainab-ali Mar 12, 2024

Baccata Mar 12, 2024

zainab-ali Mar 12, 2024

zainab-ali Mar 12, 2024

Baccata Mar 12, 2024

Baccata Mar 13, 2024

zainab-ali Mar 13, 2024

Baccata Mar 13, 2024

zainab-ali Mar 12, 2024

zainab-ali Mar 13, 2024

zainab-ali commented Mar 13, 2024

		@@ -60,7 +59,6 @@ object EffectSuite {

		}

		@RunWith(classOf[weaver.junit.WeaverRunner])

Fix CI pipeline #13

Fix CI pipeline #13

Conversation

zainab-ali commented Mar 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zainab-ali Mar 13, 2024 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zainab-ali commented Mar 13, 2024

zainab-ali commented Mar 8, 2024 •

edited

Loading

zainab-ali Mar 13, 2024 •

edited

Loading