In-source partest [ci: last-only] #6566

lrytz · 2018-04-27T09:08:39Z

No description provided.

lrytz · 2018-04-27T12:08:54Z

This is ready to be reviewed (by everyone for the change in general, @szeiger could take a special look at build.sbt).

szeiger · 2018-04-27T12:51:32Z

build.sbt

    // two lines below are copied over from sbt's sources:
    // https://github.com/sbt/sbt/blob/0.13/main/src/main/scala/sbt/Defaults.scala#L628
-    //val resolvedScalaVersion = ScalaVersion((scalaVersion in artifactName).value, (scalaBinaryVersion in artifactName).value)
+    //val resolvedScalaVersion = ScalaVersion((scalaVersion in artifactName).value, (scalapartVersion in artifactName).value)


find&replace error?

szeiger · 2018-04-27T13:01:08Z

build.sbt

-    description := "Scala Compiler Testing Tool (compiler-specific extras)",
-    libraryDependencies += partestDep,
-    unmanagedSourceDirectories in Compile := List(baseDirectory.value)
+    name := "scala-partest",


Is this intended to switch the group ID from org.scala-lang.modules to org.scala-lang? Otherwise an organization setting should be added.

You probably want to add the usual boilerplate from the other published projects, too:

.settings(generatePropertiesFileSettings) .settings(Osgi.settings) .settings(AutomaticModuleName.settings("scala.partest"))

Good question about the group ID. It seems more consistent to me to switch to org.scala-lang.

Thanks for the other suggestions.

generatePropertiesFileSettings is part of configureAsSubproject, so that's redundant (no?)

Other projects mention it, too, but it should be redundant. We can remove it everywhere.

dwijnand · 2018-05-03T14:28:49Z

Welcome back home, partest.

hrhino · 2018-05-03T14:43:31Z

Is there a reason that the partest git history isn't being preserved here? I guess the scala-partest repo will continue to exist, but I (at least) find being able to annotate files in IntelliJ to be pretty useful.

lrytz · 2018-05-03T19:21:58Z

Rebased

partest git history isn't being preserved here?

By adding another root to the git repository using git merge --allow-unrelated-histories? Any known downsides in doing this? We recently did this for the collections, see the commit message in 878e7d3.

hrhino · 2018-05-03T19:28:10Z

By adding another root to the git repository using git merge --allow-unrelated-histories?

yes, that's what I meant. I'm not aware of any downsides caused by intentional use of it; the git release note for this flag appears to imply that it's there to ensure you know what you're doing. I've merged many projects together this way at work and haven't seen any problems yet.

Besides, given how scala/scala already has two roots, that ship has sailed 😉

sjrd · 2018-05-03T19:33:06Z

FWIW, scala-js/scala-js has had two roots for 5 years without any problem. In one of the branches it shares history with https://github.com/sjrd/ozma. I don't think anyone even noticed :p

lrytz · 2018-05-03T19:57:41Z

OK, I'll probably only get to look into this next week, this week is 🌴 🏖 👨‍👩‍👧‍👦

If someone else is giving it a go, please drop a note here

szeiger · 2018-05-04T12:27:03Z

Besides, given how scala/scala already has two roots, that ship has sailed 😉

That was before we merged collection-strawman. Now it has three.

hrhino · 2018-05-04T12:40:19Z

Now it has three.

Really? Interesting. What's the other-other one?

szeiger · 2018-05-04T12:42:55Z

See 878e7d3 for the collection-strawman migration script. The gist: Pull a branch from the repo you want to merge, start a "fake" merge (git merge -s ours --allow-unrelated-histories --no-commit), put everything you want to keep from the commit that you pulled into the correct location, commit the merge.

There's a catch however: Partest was originally part of scala/scala but apparently got its history rewritten and rebased when it was pulled out into a separate repo. It should be possible to revert this by taking the first commit after the split and rebasing it on top of the version of its parent that is still in scala/scala. This will make the version that we merge back merge-incompatible with the partest repo but if we don't rebase first we end up with two merge-incompatible histories of partest in scala/scala.

szeiger · 2018-05-04T12:44:20Z

@hrhino In-sourcing the spec in #3662

dwijnand · 2018-05-04T12:44:56Z

(see git rev-list --max-parents=0 HEAD)

dwijnand · 2018-05-04T12:51:24Z

taking the first commit after the split and rebasing it on top of the version of its parent that is still in scala/scala

👍 git rebase --onto 2.13.x <first post-split commit in scala/partest> <current HEAD of scala/partest>

I documented how I re-in-sourced part of sbt/util back into sbt/sbt over in sbt/sbt#3302.

adriaanm · 2018-05-09T09:46:37Z

Is this good to go?

lrytz · 2018-05-09T12:37:06Z

Pushed the partest history that went into scala-partest between the module creation and now. So there's no new root in the git repository in the end.

Here's the log:

[scala] git log --format=oneline --full-history -- src/partest

compare the commits with the history of scala-partest
not straightforward to see where the branch happened because the scala-partest history is flat, doesn't contain merge commits
the last commit in scala-partest that also exists in scala/scala is scala/scala-partest@b3b3fd9d1d "Spin off parser combinators to scala-parser-combinators.jar", the next commit in scala-partest (scala/scala-partest@9d5205650f) moves files around and creates a build script
the corresponding commit in scala/scala is 46a4635
however, scala-partest at "spin off..." also contains scala/scala-partest@4447998 "SI-4594 Repl settings command", which is not part of scala/scala "spin off..."
Moving to a flat history in partest created a mixed commit order. Testing the waters with the merge commit of "Repl settings..", which is 3946e12 (in scala/scala)

[scala] git checkout 3946e12f0a
[partest] git checkout b3b3fd9d1d
[scala] diff -r src/partest/scala/tools/partest ~/scala/scala-partest/scala/tools/partest
[scala]

OK that looks good. So we use 3946e12 as the scala/scala commit where partest was branched off.

[partest] git checkout 1.2.x
[partest] git log --oneline -n 1 | cat
f18c249 Merge pull request #107 from scala/SethTisue-patch-1
[partest] git checkout -b inPartestHistory

Rewrite the history so that the sources are always in src/partest. Then also rewrite the history to delete everything but the src/partest/scala directory.

[partest] git filter-branch -f --prune-empty --tree-filter 'if [ -d src ]; then mkdir src/partest; mv src/main/scala/scala src/partest; else mkdir -p src/partest; mv scala src/partest; fi' inPartestHistory
[partest] git filter-branch -f --prune-empty --index-filter 'git rm --cached -r -q -- . ; git reset -q $GIT_COMMIT -- src/partest/scala' inPartestHistory

[partest] git log --oneline --grep 'Spin off parser combinators'

The branch off commit ("Spin off parser combinators") is now lrytz/scala-partest@09be915c46.

First we restore src/partest as it was when the module was branched off

[scala] git checkout -b inPartestHistory 2.13.x
[scala] git checkout 3946e12f0a -- src/partest/scala/tools/partest
[scala] git checkin

Then we add the commits from scala-partest on top:

[scala] git remote add partest-lrytz [email protected]:lrytz/scala-partest.git
[scala] git fetch partest-lrytz inPartestHistory
[scala] git rebase --onto inPartestHistory 09be915c46 partest-lrytz/inPartestHistory
[scala] git branch -f inPartestHistory

Diffing to make sure all is correct

[partest] git checkout 1.2.x
[scala] diff -r src/partest/scala/tools/partest ~/scala/scala-partest/src/main/scala/scala/tools/partest
[scala]
[scala] git diff 97dbff4430
[scala]


[scala] git cherry-pick 97dbff4430..inPartest
[scala] git diff inPartest
[scala]

Push to the PR

[scala] git checkout -b inPartestNoHistory inPartest
[scala] git push --set-upstream origin inPartestNoHistory

[scala] git checkout inPartest
[scala] git reset --hard inPartestHistory
[scala] git push -f --set-upstream origin inPartest

lrytz · 2018-05-09T12:41:52Z

Is this good to go?

I think now it is

Conventionally, a multiline file means take each line as an element, no quote parsing required. To support trailing spaces, do not aggressively trim the input; that was probably to avoid confusion over a nonempty line consistently only of spaces; but still do drop empty elements, that is, a line of zero length. An empty line is useful to force multiline handling of a single element. (A single line is split on space.)

See scala#6164 Ref scala/scala-dev#461 (still need to release partest & upgrade)

… interface

The cost of VM startup and JIT is the predominant reason that `partest --run` is slow. We're much better of using one thread on the main JVM to execute these tests. This commit uses such a mode under a non-default option. Unforked test execution requires some care: - we need to check the main JVM is started with the mandated immutable System properties (e.g. file.encoding) - other test-specific system properties (e.g. partest.output) need to be patched into the system property map before each test is executed. - sys.exit calls need to be intercepted with a security manager - stdout / stderr need to be intercepted, both in the Java and Scala console printing APIs. Some tests need to be adapted, in particular those that assume that java.class.path will contain the test classes. Tests can also force usage of the legacy forked mode by adding a .javaopts file with a dummy JVM option. Also improve recording of test durations and flag slow tests in console output.

- Comment for Stopwatch - Delete commented out debug code - Refactor properties save/restore - DRY up console scraping

We fork a process with a process logger that redirects stdout and stderr to a log file, so I can't see why this was ever needed. Tested: ``` Note: test execution will be non-parallel under -Dpartest.exec.in.process % scalac t1459/InheritingPrinter.scala t1459/JavaPrinter.java t1459/ScalaPrinter.scala t1459/Test.java t1459/VarArg.java !! 1 - run/t1459 [compilation failed] % scalac t1459/InheritingPrinter.scala t1459/JavaPrinter.java t1459/ScalaPrinter.scala t1459/Test.java t1459/VarArg.java JavaPrinter.java:1: error: illegal start of type declaration public cla ss JavaPrinter implements VarArg { ^ one error found partest --update-check \ /Users/jz/code/scala/test/files/run/t1459 ```

Fixes the regression when running under `--verbose` ``` % scalac innerClassEnclMethodJavaReflection.scala % <in process execution of jvm/innerClassEnclMethodJavaReflection.scala> > innerClassEnclMethodJavaReflection-jvm.log % diff /Users/jz/code/scala/test/files/jvm/innerClassEnclMethodJavaReflection-jvm.log /Users/jz/code/scala/test/files/jvm/innerClassEnclMethodJavaReflection.check--- innerClassEnclMethodJavaReflection-jvm.log +++ empty @@ -1,3 +1,0 @@ -% scalac unreachable/Foo_1.scala -% scalac t2470/Test_1.scala -% scalac t2470/Read_Classfile_2.scala ```

Now that scala#6255 is merged

lrytz · 2018-05-09T16:11:13Z

Start at the branching point

[scala] git checkout -b inPartestHistory 3946e12f0a

Add the commits from scala-partest

[scala] git rebase --onto 3946e12f0a 09be915c46 partest-lrytz/inPartestHistory
[scala] git branch -f inPartestHistory

[partest] git checkout 1.1.x
[scala] diff -r src/partest/scala/tools/partest ~/scala/scala-partest/src/main/scala/scala/tools/partest
[scala]

Merge into 2.13.x

[scala] git checkout -b inPartestMerge 2.13.x
[scala] git merge inPartestHistory -s ours
[scala] git checkout inPartestHistory -- src/partest/scala/tools/partest
[scala] git commit --amend --no-edit

[scala] diff -r src/partest/scala/tools/partest ~/scala/scala-partest/src/main/scala/scala/tools/partest
[scala]

Compare against the version that was initially in this PR (without history)

[scala] git diff 97dbff4430
[scala]

Get the remaining commits

[scala] git cherry-pick 97dbff4430..inPartestNoHistory
[scala] git diff inPartestNoHistory
[scala]

[scala] git checkout inPartest
[scala] git reset --hard inPartestMerge
[scala] git push -f

@szeiger does that look good?

adriaanm · 2018-05-10T13:45:55Z

(Sorry, my paradise in-source PR caused a conflict)

SethTisue · 2018-05-10T17:38:47Z

Jenkins is green, shall we merge?

lrytz · 2018-05-10T18:10:36Z

I'd like Stefan to sign it off

szeiger · 2018-05-11T13:57:32Z

LGTM

SethTisue · 2018-05-11T14:10:59Z

🎉 been wanting this for years

scala-jenkins added this to the 2.13.0-M4 milestone Apr 27, 2018

lrytz mentioned this pull request Apr 27, 2018

Make StringOps a pure value class #6565

Merged

lrytz force-pushed the inPartest branch from 866925b to d90d79a Compare April 27, 2018 11:33

lrytz changed the title ~~In-source partest [WIP] [ci: last-only]~~ In-source partest [ci: last-only] Apr 27, 2018

szeiger reviewed Apr 27, 2018

View reviewed changes

lrytz force-pushed the inPartest branch 2 times, most recently from 895f46d to e02c2ca Compare April 27, 2018 14:46

szeiger mentioned this pull request Apr 30, 2018

Add missing operations to ArrayOps #6572

Merged

szeiger approved these changes May 3, 2018

View reviewed changes

This was referenced May 3, 2018

Remove --self-test (not used) scala/scala-partest#108

Closed

switch to sbt-git scala/scala-partest#36

Closed

lrytz force-pushed the inPartest branch from 7a84c3e to ab30ea2 Compare May 3, 2018 19:12

lrytz force-pushed the inPartest branch 2 times, most recently from d3c5a60 to ffa2fbd Compare May 9, 2018 12:32

lrytz and others added 19 commits May 9, 2018 17:41

Avoid deprecation

6114993

Remove unused import

7c9015f

Avoid scala.sys.process.javaVmArguments

734b2a2

See scala#6164 Ref scala/scala-dev#461 (still need to release partest & upgrade)

Propagate exceptions and failure transcripts to the SBT test listener…

a4d51c5

… interface

Pass the srcpath as FQN, test file as testSelector in SBT test events

f4ccab2

Address review feedback

0cd8139

- Comment for Stopwatch - Delete commented out debug code - Refactor properties save/restore - DRY up console scraping

Use a store fence to try to avoid race condition with sys.props access

47e2a82

Implicit conversions from Array to Seq is deprecated in 2.13

1fc89b2

Remove ant support

e2a211c

Now that scala#6255 is merged

Merge branch 'inPartestHistory' into inPartestMerge

f599e95

Move files from partest-extras into partest

63c6d92

Update build for in-sourced partest

e3806a2

IntelliJ structure

9241b57

Publish scala-partest artifact

4c604fd

remove unused code for partest self-tests

71007d1

lrytz force-pushed the inPartest branch from ffa2fbd to 71007d1 Compare May 9, 2018 16:08

scala deleted a comment from adriaanm May 10, 2018

Merge branch '2.13.x' into inPartest

e253ad1

szeiger merged commit 6901521 into scala:2.13.x May 11, 2018

SethTisue mentioned this pull request Oct 23, 2023

Classloader for macros is misbehaving scala/bug#5433

Closed

In-source partest [ci: last-only] #6566

In-source partest [ci: last-only] #6566

Uh oh!

Conversation

lrytz commented Apr 27, 2018

Uh oh!

lrytz commented Apr 27, 2018

Uh oh!

szeiger Apr 27, 2018

Choose a reason for hiding this comment

Uh oh!

szeiger Apr 27, 2018

Choose a reason for hiding this comment

Uh oh!

lrytz Apr 27, 2018

Choose a reason for hiding this comment

Uh oh!

lrytz Apr 27, 2018

Choose a reason for hiding this comment

Uh oh!

szeiger Apr 30, 2018

Choose a reason for hiding this comment

Uh oh!

dwijnand commented May 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hrhino commented May 3, 2018

Uh oh!

lrytz commented May 3, 2018

Uh oh!

hrhino commented May 3, 2018

Uh oh!

sjrd commented May 3, 2018

Uh oh!

lrytz commented May 3, 2018

Uh oh!

szeiger commented May 4, 2018

Uh oh!

hrhino commented May 4, 2018

Uh oh!

szeiger commented May 4, 2018

Uh oh!

szeiger commented May 4, 2018

Uh oh!

dwijnand commented May 4, 2018

Uh oh!

dwijnand commented May 4, 2018

Uh oh!

adriaanm commented May 9, 2018

Uh oh!

lrytz commented May 9, 2018

Uh oh!

lrytz commented May 9, 2018

Uh oh!

lrytz commented May 9, 2018

Uh oh!

adriaanm commented May 10, 2018

Uh oh!

SethTisue commented May 10, 2018

Uh oh!

lrytz commented May 10, 2018

Uh oh!

szeiger commented May 11, 2018

Uh oh!

SethTisue commented May 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

17 participants

dwijnand commented May 3, 2018 •

edited

Loading