Skip to content

Run tests for partest #554

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 4 commits into from
May 12, 2015
Merged

Run tests for partest #554

merged 4 commits into from
May 12, 2015

Conversation

vsalvis
Copy link
Contributor

@vsalvis vsalvis commented May 10, 2015

Added runFile/runDir methods to CompilerTest. A run test needs to contain an

object Test { 
  def main(args: Array[String]): Unit = ... // print something
}

and a testname.check file which needs to match the output of the run test.

@DarkDimius
Copy link
Contributor

A run test needs to contain an

Should test contain the main method? or should it also be Test?

@vsalvis
Copy link
Contributor Author

vsalvis commented May 10, 2015

Not sure I understand your question... The code being run by partest is the main method of the Test object, so the "..." in the code above...

@DarkDimius
Copy link
Contributor

I mean, if I name object not Test but Test2, and it will be in other package than <empty>, will partest still run it?

@vsalvis
Copy link
Contributor Author

vsalvis commented May 10, 2015

No, it runs exactly the main method of the Test object, nothing else. It
fails with "no such file: Test" if the object isn't there and
"NoSuchMethodException: Test.main" if there's no main.
On May 10, 2015 10:06 PM, "Dmitry Petrashko" [email protected]
wrote:

I mean, if I name object not Test but Test2, and it will be in other
package than , will partest still run it?


Reply to this email directly or view it on GitHub
#554 (comment).

@DarkDimius
Copy link
Contributor

Good to know. Thanks.

try {
partestLock = new RandomAccessFile(partestLockFile, "rw").getChannel.tryLock
} catch {
case ex: java.nio.channels.OverlappingFileLockException => // locked already
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure it is locked by same SBT instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This exception is thrown when the lock is held by the same JVM. This could happen if Tests.Cleanup doesn't run for some reason. If the lock is held by another JVM, tryLock returns null. In both cases, we just continue because the file is locked (even though not by us). In general it's a bad idea to have two sbt instances run tests at the same time since we only communicate partest-ness through the file locking, which is a global property, not a per-sbt property. So maybe it would be better to fail if we can't lock the file?

The JUnit test switches between partest file generation and running the tests based on the lock file. So if one sbt instance does partest and the other test, then the second will still generate partest files (because the file is still locked at the time when the JUnit test runs). So in the second case, the tests are neither run by JUnit nor by partest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added exceptions in those two cases (commit below), however running test while a concurrent partest is running will not throw an exception and result in the undesired behavior described above, and depending on the timing other interleavings of test & partest could mess things up as well... So generally one should avoid running tests in two concurrent sbt instances.

@vsalvis vsalvis force-pushed the vsalvis-partest-run branch from bf6ce11 to f64762b Compare May 12, 2015 09:48
DarkDimius added a commit that referenced this pull request May 12, 2015
@DarkDimius DarkDimius merged commit a0fa33d into scala:master May 12, 2015
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