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

fix: tolerate endo pre and post #822 #3472

Merged
merged 2 commits into from
Jul 14, 2021
Merged

Conversation

erights
Copy link
Member

@erights erights commented Jul 13, 2021

Test was testing for error message before endojs/endo#822 , which is not yet part of an endo release. In anticipation, and to aid development against endo master using yarn link, temporarily make it tolerate both the old and new error messages.

Question for reviewers: To what degree should tests be testing for the content of error messages anyway?

@erights erights requested review from kriskowal and dckc July 13, 2021 23:57
@erights erights self-assigned this Jul 13, 2021
Copy link
Member

@kriskowal kriskowal left a comment

Choose a reason for hiding this comment

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

I’m generally not fond of “change detector” tests. There should be a message test at least to be sure the error is related to the intended failure. A very small substring should suffice.

@erights erights merged commit e872c0c into master Jul 14, 2021
@erights erights deleted the markm-tolerate-endo-822 branch July 14, 2021 00:45
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