Skip to content

SI-6502 Convert test to asserts #9

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 1 commit into from
Jan 21, 2015

Conversation

som-snytt
Copy link

This saves a check file in the crowded test directory.

This saves a check file in the crowded test directory.
@som-snytt
Copy link
Author

Not trying to be obnoxious with this PR.

Actually, I wanted to do something like the following, but it says "class not found":

object Test extends ReplTest {
  def code = {
    testOutput.jfile.mkdirs()
    s"""
      |:reset -Yrepl-outdir ${jarnr(1)}
      |:pa -raw
      |package test { object Test { def test() = println("testing...") } }
      |\u0004
      |:reset -Yrepl-outdir ${testOutput.path}
      |:require ${jarnr(1)}
      |test.Test.test()
      |
      |
    """.stripMargin
  }

  def jarnr(n: Int) = s"${testOutput.toAbsolute.path}/test${n}.jar"
  [etc]

That works in the REPL, so it should work under partest.

I had to tweak the paste code to handle the "raw" ctl-D, but otherwise, not sure why it didn't work the first time.

@retronym
Copy link
Owner

Cheers!

retronym added a commit that referenced this pull request Jan 21, 2015
SI-6502 Convert test to asserts
@retronym retronym merged commit c6c9565 into retronym:ticket/6502 Jan 21, 2015
retronym added a commit that referenced this pull request Jul 31, 2015
Calls to synthetic case class apply methods are inlined to the
underlying constructor invocation in refchecks.

However, this can lead to accessibility errors if the constructor
is private.

This commit ensures that the constructor is at least as accessible
as the apply method before performing this tranform.

I've manually checked that other the optimization still works in other
cases:

scala> class CaseApply { Some(42)  }
defined class CaseApply

    scala> :javap -c CaseApply
    Compiled from "<console>"
    public class CaseApply {
      public CaseApply();
        Code:
           0: aload_0
           1: invokespecial #9                  // Method java/lang/Object."<init>":()V
           4: new           #11                 // class scala/Some
           7: dup
           8: bipush        42
          10: invokestatic  #17                 // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
          13: invokespecial #20                 // Method scala/Some."<init>":(Ljava/lang/Object;)V
          16: pop
          17: return
    }
retronym added a commit that referenced this pull request Jul 31, 2015
Calls to synthetic case class apply methods are inlined to the
underlying constructor invocation in refchecks.

However, this can lead to accessibility errors if the constructor
is private.

This commit ensures that the constructor is at least as accessible
as the apply method before performing this tranform.

I've manually checked that other the optimization still works in other
cases:

scala> class CaseApply { Some(42)  }
defined class CaseApply

    scala> :javap -c CaseApply
    Compiled from "<console>"
    public class CaseApply {
      public CaseApply();
        Code:
           0: aload_0
           1: invokespecial #9                  // Method java/lang/Object."<init>":()V
           4: new           #11                 // class scala/Some
           7: dup
           8: bipush        42
          10: invokestatic  #17                 // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
          13: invokespecial #20                 // Method scala/Some."<init>":(Ljava/lang/Object;)V
          16: pop
          17: return
    }
@som-snytt som-snytt deleted the review/6502 branch June 27, 2019 04:28
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.

2 participants