Skip to content

Golden tests for purs/failing and purs/warning #3774

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
merged 3 commits into from
Feb 22, 2020

Conversation

dariooddenino
Copy link
Contributor

No description provided.

@hdgarrood
Copy link
Contributor

On second thoughts I think I'd prefer to go with what @natefaubion suggested on Discourse and keep the shouldFailWith stuff in here, just in case. The code probably isn't doing much harm. (Also, as an aside, if we were going to remove the code for handling these special comments, then we should also remove all of the special comments from the test input files too.)

@dariooddenino
Copy link
Contributor Author

You mean that you would like to have both the golden tests and the directives check? I thought @natefaubion meant that he just wanted the directives on the test files, not the tests.

@hdgarrood
Copy link
Contributor

I’m pretty sure the idea was to keep in both the directives themselves and the code which tests them; they aren’t very useful if we aren’t checking them.

@garyb
Copy link
Member

garyb commented Jan 24, 2020

I'm a fan of keeping the shouldFailWith etc. because otherwise these tests are only useful for preventing regressions. When working on an error or warning I tend to work in a TDDish way, in that I make a bunch of cases that should fail first and then work on making them fail the correct way after.

@dariooddenino
Copy link
Contributor Author

Directives are back in :)

@natefaubion
Copy link
Contributor

Do we want to omit ANSI color sequences in the .out file?

@hdgarrood
Copy link
Contributor

I don't have any strong feelings on this - I guess would make the test output a bit nicer to look at if we did omit them, but also the colours might be worth testing as they do help distinguish names in your code from error message text, as seen in eg #2056

Copy link
Contributor

@hdgarrood hdgarrood left a comment

Choose a reason for hiding this comment

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

Looks great. Thank you very much!

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