Skip to content

Fix printed file path in PrintingTest.scala #8330

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

Merged
merged 2 commits into from
Feb 18, 2020
Merged

Fix printed file path in PrintingTest.scala #8330

merged 2 commits into from
Feb 18, 2020

Conversation

michelou
Copy link
Contributor

This fix solves one test case among several tests still failing on Windows (Win10/Java 8).

> sbt "testOnly dotty.tools.dotc.PrintingTest"
[info] Loading settings for project dotty-build-build from build.sbt ...
[info] Loading project definition from W:\dotty\project\project
[info] Loading settings for project dotty-build from build.sbt,plugins.sbt ...
[info] Loading project definition from W:\dotty\project
[info] Loading settings for project dotty from build.sbt ...
[info] Resolving key references (25620 settings) ...
[info] Set current project to dotty (in build file:/W:/dotty/)
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for dotty-interfaces / Test / testOnly
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for dotty-library / Test / testOnly
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for tasty-core / Test / testOnly
[warn] Multiple main classes detected.  Run 'show discoveredMainClasses' to see the list
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for Test / testOnly
[info] Compiling 1 Scala source to W:\dotty\compiler\target\scala-0.22\test-classes ...
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for dotty-sbt-bridge / Test / testOnly
[info] Test run started
[info] Test dotty.tools.dotc.PrintingTest.printing started
Output from 'tests\printing\i620.scala' did not match check file. Actual output:
result of tests\printing\i620.scala after typer:
package O {
  package O.A {
    class D() extends Object() {
      class C() extends Object() {
        protected[D] def a: Int = 0
        private[D] def b: Int = 0
        private[this] def c: Int = 0
        protected def d: Int = 0
        private[A] def e: Int = 0
        protected[A] def f: Int = 0
        def g: Int = 0
        def g1(t: Int): Int = 0
        def (c: D.this.C) g1: Int = 0
      }
      private[D] class E() extends Object() {}
      private[this] class F() extends Object() {}
      private[A] class G() extends Object() {}
      protected[D] class H() extends Object() {}
      protected class I() extends Object() {}
      protected[A] class J() extends Object() {}
      class K() extends Object() {}
      protected[D] val a: Int = 0
      private[D] val b: Int = 0
      private[this] val c: Int = 0
      protected val d: Int = 0
      private[A] val e: Int = 0
      protected[A] val f: Int = 0
      val g: Int = 0
    }
  }
}


[error] Test dotty.tools.dotc.PrintingTest.printing failed: java.lang.AssertionError: assertion failed: Pass: 0, Failed: 1, took 4.0 sec
Test output dumped in: tests\printing\i620.check.out
  See diff of the checkfile (`brew install icdiff` for colored diff)
    > diff tests\printing\i620.check tests\printing\i620.check.out
  Replace checkfile with current output
    > mv tests\printing\i620.check.out tests\printing\i620.check

[error]     at dotty.DottyPredef$.assertFail(DottyPredef.scala:17)
[error]     at dotty.tools.dotc.PrintingTest.printing(PrintingTest.scala:59)
[error]     ...
[info] Test run finished: 1 failed, 0 ignored, 1 total, 4.391s
[error] Failed: Total 1, Failed 1, Errors 0, Passed 0
[error] Failed tests:
[error]         dotty.tools.dotc.PrintingTest
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for dotty-doc / Test / testOnly
[error] (dotty-compiler / Test / testOnly) sbt.TestsFailedException: Tests unsuccessful
[error] Total time: 19 s, completed 11 Feb 2020 21:28:11

On Windows both files differ only in the first line for test i620.scala :

  • 1st line in check file: result of tests/printing/i620.scala after typer:
  • 1st line in output file: result of tests\printing\i620.scala after typer:

Replacing selectively the file separator on Windows does solve the issue:

> sbt "testOnly dotty.tools.dotc.PrintingTest"
[info] Loading settings for project dotty-i620-build-build from build.sbt ...
[info] Loading project definition from W:\dotty-i620\project\project
[info] Loading settings for project dotty-i620-build from build.sbt,plugins.sbt ...
[info] Loading project definition from W:\dotty-i620\project
[info] Loading settings for project dotty from build.sbt ...
[info] Resolving key references (25620 settings) ...
[info] Set current project to dotty (in build file:/W:/dotty-i620/)
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for dotty-library / Test / testOnly
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for dotty-interfaces / Test / testOnly
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for tasty-core / Test / testOnly
[warn] Multiple main classes detected.  Run 'show discoveredMainClasses' to see the list
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for Test / testOnly
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for dotty-sbt-bridge / Test / testOnly
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for dotty-doc / Test / testOnly
[info] Test run started
[info] Test dotty.tools.dotc.PrintingTest.printing started
Pass: 1, Failed: 0
[info] Test run finished: 0 failed, 0 ignored, 1 total, 4.0s
[info] Passed: Total 1, Failed 0, Errors 0, Passed 1
[success] Total time: 12 s, completed 11 Feb 2020 21:25:30

@michelou michelou requested a review from liufengyun February 17, 2020 11:28
Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, LGTM

FileDiff.dump(outFilePath, actualLines)
println(msg)
println(FileDiff.diffMessage(checkFilePath, outFilePath))
false
case _ =>
val jOutFilePath = Paths.get(outFilePath)
if (Files.exists(jOutFilePath))
try { Files.delete(jOutFilePath) } catch { case _: Exception => () }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will happen if we leave out the code here? If they are really needed, it seems the best place is inside the method dump? BTW, Files. deleteIfExists will make the code shorter (maybe remove the try/catch).

Copy link
Contributor Author

@michelou michelou Feb 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a scenario to explain why I would not leave out the highlighted code.

Given the test case from command sbt "testOnly dotty.tools.dotc.PrintingTest",

  1. I run the above command and it fails.
    I can compare the output file (suffx .check.out) with the check file (suffix .check).
  2. I fix the issue (and compile the modified source files).
  3. I rerun the above command and it succeeds.
    The output file (suffx .check.out) is still there (generated by step 1) despite a successful execution.

Yes, using Files.deleteIfExists is better.

@liufengyun liufengyun merged commit 5018a12 into scala:master Feb 18, 2020
@liufengyun
Copy link
Contributor

Thanks for the contribution @michelou 🎉

@michelou michelou deleted the dotty-i620 branch February 18, 2020 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants