Skip to content
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

Neg tests check files for // error markers #720

Closed
wants to merge 3 commits into from

Conversation

vsalvis
Copy link
Contributor

@vsalvis vsalvis commented Jul 10, 2015

Neg tests run by JUnit now check that for each compiler error, there's a // error marker on the corresponding line in the source file. For errors that don't have a source position, add // nopos-error to any of the files (for an example of this, see instantiateAbstract).

Also FYI I had one case where not all errors get displayed by the compiler, maybe if there are more than two errors for a given line? In this file there are three errors displayed, but four errors total, and the test gets all four:

[info] Test dotc.tests.neg_autoTupling started
./tests/pos/autoTuplingTest.scala:3: error: too many arguments for method apply: (x: (A? >: Int(1)))Some[(A? >: Int(1))]
  val x = Some(1, 2)                                  // error when running with -language:noAutoTupling
                  ^
./tests/pos/autoTuplingTest.scala:6: error: wrong number of argument patterns for Some; expected: (Nothing)
    case Some(a, b) => a + b                          // error // error when running with -language:noAutoTupling
             ^
./tests/pos/autoTuplingTest.scala:6: error: not found: b
    case Some(a, b) => a + b                          // error // error when running with -language:noAutoTupling
                           ^
four errors found

=== found: List(./tests/pos/autoTuplingTest.scala:3)
=== found: List(./tests/pos/autoTuplingTest.scala:6, ./tests/pos/autoTuplingTest.scala:6, ./tests/pos/autoTuplingTest.scala:6)
=== all errors:
class dotty.tools.dotc.reporting.Reporter$Error at ./tests/pos/autoTuplingTest.scala:6: type mismatch:
 found   : Nothing(a)
 required: ?{ + : ? }
Note that implicit conversions cannot be applied because they are ambiguous;
 both method Double2double in object Predef$ and method unaugmentString in object Predef$ convert from Nothing(a) to ?{ + : FunProto(b):? }
class dotty.tools.dotc.reporting.Reporter$Error at ./tests/pos/autoTuplingTest.scala:6: not found: b
class dotty.tools.dotc.reporting.Reporter$Error at ./tests/pos/autoTuplingTest.scala:6: wrong number of argument patterns for Some; expected: (Nothing)
class dotty.tools.dotc.reporting.Reporter$Error at ./tests/pos/autoTuplingTest.scala:3: too many arguments for method apply: (x: (A? >: Int(1)))Some[(A? >: Int(1))]

Do we plan on running neg tests on partest? In that case I have to add the functionality there as well, for now it's just the JUnit tests, while partest only checks the xerrors as before.

@vsalvis vsalvis force-pushed the vsalvis-neg-lines branch from acd7306 to 955cbe7 Compare July 20, 2015 14:11
@vsalvis
Copy link
Contributor Author

vsalvis commented Jul 20, 2015

Thank you very much for your comments. I restructured the code to start by reading the files, and then check the two error categories independently. Hopefully it's clearer now.

@@ -210,6 +210,8 @@ abstract class Reporter {
var warningCount = 0
def hasErrors = errorCount > 0
def hasWarnings = warningCount > 0
private var errors: List[Error] = Nil
def allErrors = errors
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not super-happy with recording all errors here. That should be the job of individual reporters, not of the base class Reporter. I believe it would be better to install a particular reporter that stores the errors in the test framework.

@DarkDimius
Copy link
Contributor

@vsalvis, do you want to give it a shot and try to make a reporter that records errors? Otherwise I'd merge this to make new tests use this machinery.

@vsalvis
Copy link
Contributor Author

vsalvis commented Jul 28, 2015

I'm in a dance camp, so I don't think it's going to happen this week. But I
can do it next week, or you can merge already and I'll add it by August 16
when I'm back from Serbia/Croatia.

On Mon, Jul 27, 2015 at 4:19 PM, Dmitry Petrashko notifications@github.com
wrote:

@vsalvis https://github.com/vsalvis, do you want to give it a shot and
try to make a reporter that records errors? Otherwise I'd merge this to
make new tests use this machinery.


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

@VladimirNik
Copy link
Contributor

Rebased version: #1106

OlivierBlanvillain pushed a commit to OlivierBlanvillain/dotty that referenced this pull request Dec 12, 2016
Lazy fields null out fields that are used only in their initializer.
When the lazy value is forced, it will null out all private fields that
are used only by the current lazy value. This should reduce memory
leaks, see scala#720
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.

4 participants