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

Improve test coverage after #1256. #1286

Merged
merged 2 commits into from
Jun 9, 2022
Merged

Improve test coverage after #1256. #1286

merged 2 commits into from
Jun 9, 2022

Conversation

zygoloid
Copy link
Contributor

This change updated a test to use a different example to make sure a
diagnostic was still being exercised, but as a consequence lost test
coverage of the situation the test was originally intended to cover.

Add back the old test and put the new test into a different file.

This change updated a test to use a different example to make sure a
diagnostic was still being exercised, but as a consequence lost test
coverage of the situation the test was originally intended to cover.

Add back the old test and put the new test into a different file.
@zygoloid zygoloid requested a review from pk19604014 May 23, 2022 23:33
@zygoloid zygoloid requested a review from a team as a code owner May 23, 2022 23:33
Copy link
Contributor

@pk19604014 pk19604014 left a comment

Choose a reason for hiding this comment

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

Thanks!


package ExplorerTest api;

// CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/impl/fail_impl_as_non_interface.carbon:[[@LINE+1]]: expected constraint after `as`, found value of type String
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any concern that the error message is bad, in that it says "value of type String" instead of "String" or "value of type Type"?

interface Vector(Scalar:! Type) {
}

// CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/impl/fail_impl_as_parameterized.carbon:[[@LINE+1]]: Expected a type, but got Vector
Copy link
Contributor

Choose a reason for hiding this comment

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

This error message seems wrong too -- a type isn't expected in place of "Vector".

Copy link
Contributor

@jsiek jsiek left a comment

Choose a reason for hiding this comment

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

Glad that you're restoring the test coverage.

@zygoloid zygoloid merged commit 92c8aa7 into carbon-language:trunk Jun 9, 2022
chandlerc pushed a commit that referenced this pull request Jun 28, 2022
This change updated a test to use a different example to make sure a
diagnostic was still being exercised, but as a consequence lost test
coverage of the situation the test was originally intended to cover.

Add back the old test and put the new test into a different file.
@chandlerc chandlerc added the explorer Action items related to Carbon explorer code label Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
explorer Action items related to Carbon explorer code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants