X10
  1. X10
  2. XTENLANG-1244

Compiler support for regression testing

    Details

    • Number of attachments :
      0

      Description

      The current approach of marking tests that are supposed to fail compilation with the _MustFailCompile suffix is too coarse-grained, because there is no way to specify a specific error to check for. So some _MustFailCompile tests passed for the wrong reasons (i.e., unrelated compilation errors).

      I suggest we enlist compiler support in checking for specific errors. The compiler could succeed (and produce a pre-defined file, or simply remove the offending code from the original test case) if a particular error is present on its error queue at the end of the compilation (or, better yet, if the specified set of errors is exactly what's on the compiler's error queue).

      The test infrastructure already supports adding per-test compile options. We could make use of this feature to specify the expected errors to the compiler. For example, we could add an -ENSURE_ERROR=id option to check for a specific error identifier, and, as an interim measure, an -ENSURE_ERROR_MESSAGE="message" to check that a particular message pattern appears on the error queue. The _MustFailCompile would be removed from the test names, and the compiler will ensure that compilation succeeds appropriately if the specified error is (or would have been) present, and the resulting code will be something that runs normally (perhaps simply a run() method with return true; as the body).

      Until this is done, we will have "accidental" successes of _MustFailCompile tests.

        Issue Links

          Activity

          David Grove made changes -
          Field Original Value New Value
          Fix Version/s X10 2.1 [ 15896 ]
          Fix Version/s X10 2.0.4 [ 16248 ]
          Igor Peshansky made changes -
          Original Estimate 3 weeks [ 1814400 ]
          Remaining Estimate 3 weeks [ 1814400 ]
          Hide
          Igor Peshansky added a comment -

          This is not a release blocker. Defer to 2.1.

          Show
          Igor Peshansky added a comment - This is not a release blocker. Defer to 2.1.
          Igor Peshansky made changes -
          Fix Version/s X10 2.1.0 [ 16496 ]
          Fix Version/s X10 2.0.5 [ 15896 ]
          Hide
          Bard Bloom added a comment -

          Sounds good to me, but I think it's a front-end sort of issue.

          Show
          Bard Bloom added a comment - Sounds good to me, but I think it's a front-end sort of issue.
          Bard Bloom made changes -
          Assignee Bard Bloom [ bard ] Igor Peshansky [ ipeshansky ]
          Hide
          Igor Peshansky added a comment -

          It will indeed become a frontend issue once we have a design for this.

          Show
          Igor Peshansky added a comment - It will indeed become a frontend issue once we have a design for this.
          Igor Peshansky made changes -
          Assignee Igor Peshansky [ ipeshansky ] Bard Bloom [ bard ]
          Hide
          Igor Peshansky added a comment -

          Yoav has been working on validating the successes of _MustFailCompile tests, so can testify how easy it is to get spurious passes on those. However, this should not block the 2.1.0 release. Defer to 2.1.1.

          Show
          Igor Peshansky added a comment - Yoav has been working on validating the successes of _MustFailCompile tests, so can testify how easy it is to get spurious passes on those. However, this should not block the 2.1.0 release. Defer to 2.1.1.
          Igor Peshansky made changes -
          Fix Version/s X10 2.1.1 [ 16497 ]
          Fix Version/s X10 2.1.0 [ 16496 ]
          Hide
          Bard Bloom added a comment -

          Yoav has, I believe, greatly improved testing. He can decide if it's sufficient for our purposes; I suspect it probably is.

          Show
          Bard Bloom added a comment - Yoav has, I believe, greatly improved testing. He can decide if it's sufficient for our purposes; I suspect it probably is.
          Bard Bloom made changes -
          Assignee Bard Bloom [ bard ] Yoav Zibin [ yzibin ]
          Hide
          Yoav Zibin added a comment -

          The design is simple:
          I have 3 kind of markers:

          "// ... ERR"  - marks an error
          "// ... ShouldNotBeERR" - the compiler reports an error, but it shouldn't
          "// ... ShouldBeErr"- the compiler doesn't report an error, but it should
          

          I check that there is an error in that line (but I don't check a specific error message, because those change often).
          So it reduces the risk of an accidental passing test.

          See RunTestSuite.

          Show
          Yoav Zibin added a comment - The design is simple: I have 3 kind of markers: " // ... ERR" - marks an error " // ... ShouldNotBeERR" - the compiler reports an error, but it shouldn't " // ... ShouldBeErr" - the compiler doesn't report an error, but it should I check that there is an error in that line (but I don't check a specific error message, because those change often). So it reduces the risk of an accidental passing test. See RunTestSuite .
          Hide
          Yoav Zibin added a comment - - edited

          This is the design:
          I added 3 kinds of annotations:

          @ERR
          @ShouldNotBeERR
          @ShouldBeErr
          

          I also added another phase called:
          ErrChecker
          that is on when passing the flag CHECK_ERR_MARKERS.

          I also added to ErrorInfo 3 new {{kind}}s:

            public static final int ERR_MARKER        = 8;  // A line was marked with @ERR
            public static final int SHOULD_NOT_BE_ERR_MARKER    = 9;  // A line was marked with @ShouldNotBeERR
            public static final int SHOULD_BE_ERR_MARKER    = 10; // A line was marked with @ShouldBeErr
          

          The phase ErrChecker does the following:
          it iterates over all the annotations, searching for one of the above 3 annotations.
          then it issues the corresponding error in that line position.

          RunTestSuite takes all errors and compares them with errors issued by the compiler.
          Read the documentation in RunTestSuite.

          Show
          Yoav Zibin added a comment - - edited This is the design: I added 3 kinds of annotations: @ERR @ShouldNotBeERR @ShouldBeErr I also added another phase called: ErrChecker that is on when passing the flag CHECK_ERR_MARKERS . I also added to ErrorInfo 3 new {{kind}}s: public static final int ERR_MARKER = 8; // A line was marked with @ERR public static final int SHOULD_NOT_BE_ERR_MARKER = 9; // A line was marked with @ShouldNotBeERR public static final int SHOULD_BE_ERR_MARKER = 10; // A line was marked with @ShouldBeErr The phase ErrChecker does the following: it iterates over all the annotations, searching for one of the above 3 annotations. then it issues the corresponding error in that line position. RunTestSuite takes all errors and compares them with errors issued by the compiler. Read the documentation in RunTestSuite .
          Yoav Zibin made changes -
          Assignee Yoav Zibin [ yzibin ] Igor Peshansky [ ipeshansky ]
          Hide
          David Grove added a comment -

          bulk move of all unresolved issues from 2.1.1 to 2.1.2

          Show
          David Grove added a comment - bulk move of all unresolved issues from 2.1.1 to 2.1.2
          David Grove made changes -
          Fix Version/s X10 2.1.2 [ 16498 ]
          Fix Version/s X10 2.1.1 [ 16497 ]
          Hide
          David Grove added a comment -

          defer remaining 2.1.2 issues to 2.2.0

          Show
          David Grove added a comment - defer remaining 2.1.2 issues to 2.2.0
          David Grove made changes -
          Fix Version/s X10 2.2 [ 16002 ]
          Fix Version/s X10 2.1.2 [ 16498 ]
          Hide
          Igor Peshansky added a comment -

          Yoav, let's figure out if we're done with this issue and need to change the nightly test scripts, or if there's more work to be done on this in the frontend.

          Show
          Igor Peshansky added a comment - Yoav, let's figure out if we're done with this issue and need to change the nightly test scripts, or if there's more work to be done on this in the frontend.
          Igor Peshansky made changes -
          Assignee Igor Peshansky [ ipeshansky ] Yoav Zibin [ yzibin ]
          Hide
          Igor Peshansky added a comment -

          Ongoing work. Defer to 2.2.1.

          Show
          Igor Peshansky added a comment - Ongoing work. Defer to 2.2.1.
          Igor Peshansky made changes -
          Fix Version/s X10 2.2.1 [ 17131 ]
          Fix Version/s X10 2.2 [ 16002 ]
          Hide
          David Grove added a comment -

          bulk defer of open issues to 2.2.2.

          Show
          David Grove added a comment - bulk defer of open issues to 2.2.2.
          David Grove made changes -
          Fix Version/s X10 2.2.2 [ 17639 ]
          Fix Version/s X10 2.2.1 [ 17131 ]
          Hide
          David Grove added a comment -

          Need to determine what else needs to be done to be able to close this issue. Igor, could you summarize?

          Show
          David Grove added a comment - Need to determine what else needs to be done to be able to close this issue. Igor, could you summarize?
          David Grove made changes -
          Link This issue is depended upon by XTENLANG-2971 [ XTENLANG-2971 ]
          Hide
          Yoav Zibin added a comment -

          I think you can close it.
          If you run with RunTestSuite you can have specific error markers on the exact line that should fail compilation.
          (Although it is not a specific error message, writing "// ERR" on the desired line is specific enough.)

          Show
          Yoav Zibin added a comment - I think you can close it. If you run with RunTestSuite you can have specific error markers on the exact line that should fail compilation. (Although it is not a specific error message, writing "// ERR" on the desired line is specific enough.)
          Hide
          Igor Peshansky added a comment -

          We've had too many of these succeed with the wrong error (e.g., a syntax error instead of a type error, etc). I'd be more comfortable with having a more precise control over the expected messages.

          There was some work on assigning error codes to messages. Has that progressed at all? If we had a way of specifying the expected code (e.g., // ERR:T2194 for type error with code 2194), the checking would be much more precise, and this issue could be closed.

          Show
          Igor Peshansky added a comment - We've had too many of these succeed with the wrong error (e.g., a syntax error instead of a type error, etc). I'd be more comfortable with having a more precise control over the expected messages. There was some work on assigning error codes to messages. Has that progressed at all? If we had a way of specifying the expected code (e.g., // ERR:T2194 for type error with code 2194), the checking would be much more precise, and this issue could be closed.
          David Grove made changes -
          Fix Version/s X10 2.2.3 [ 18146 ]
          Fix Version/s X10 2.2.2 [ 17639 ]
          David Grove made changes -
          Assignee Yoav Zibin [ yzibin ]
          David Grove made changes -
          Fix Version/s X10 2.3 [ 17009 ]
          Fix Version/s X10 2.2.3 [ 18146 ]
          David Grove made changes -
          Fix Version/s X10 2.4 [ 17010 ]
          Fix Version/s X10 2.3 [ 17009 ]
          David Grove made changes -
          Priority Critical [ 2 ] Major [ 3 ]
          Hide
          David Grove added a comment -

          I'm declaring this as fixed as it is likely to be.

          When we get focus to work on this again, we can use a new issue.

          Show
          David Grove added a comment - I'm declaring this as fixed as it is likely to be. When we get focus to work on this again, we can use a new issue.
          David Grove made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          David Grove made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Unassigned
              Reporter:
              Igor Peshansky
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 3 weeks
                3w
                Remaining:
                Remaining Estimate - 3 weeks
                3w
                Logged:
                Time Spent - Not Specified
                Not Specified