Skip to content

Fix #1965: add proper testing infrastructure for reporting tests #1966

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

Conversation

felixmulder
Copy link
Contributor

No description provided.

@felixmulder felixmulder added area:reporting Error reporting including formatting, implicit suggestions, etc itype:enhancement prio:high labels Feb 10, 2017
}
}

ctx = freshReporter(ctx)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would move the mutation stuff at the very top of the trait. BTW could you make it a class instead? (it feels wrong to have class → trait → class inheritance)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe DottyTest should be a trait instead?

f(ictx, messages)
}

def expectNoErrors: Unit = this.isInstanceOf[FailedReport]
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean asInstanceOf?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, but I should probably rename FailedReport => EmptyReport

Copy link
Contributor

Choose a reason for hiding this comment

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

Then I don't understand the statement, this computes a Boolean and converts it to Unit regardless of it's value?

rep
}

def messageCount(expected: Int, messages: List[Message]): Unit =
Copy link
Contributor

Choose a reason for hiding this comment

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

assertMessageCount

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@OlivierBlanvillain
Copy link
Contributor

Looks good otherwise :)

felixmulder added a commit to dotty-staging/dotty that referenced this pull request Feb 10, 2017
@felixmulder felixmulder force-pushed the topic/error-messages-unit-tests branch from 21bc19e to 3d20e9d Compare February 10, 2017 14:39
felixmulder added a commit to dotty-staging/dotty that referenced this pull request Feb 10, 2017
@felixmulder felixmulder force-pushed the topic/error-messages-unit-tests branch from 3d20e9d to 07384ca Compare February 10, 2017 14:42
@felixmulder felixmulder merged commit e9c4942 into scala:master Feb 10, 2017
@felixmulder felixmulder deleted the topic/error-messages-unit-tests branch February 10, 2017 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:reporting Error reporting including formatting, implicit suggestions, etc itype:enhancement prio:high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants