Skip to content

Add test for errors involving module classes #6487

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 2 commits into from
May 10, 2019

Conversation

odersky
Copy link
Contributor

@odersky odersky commented May 9, 2019

Ideally, we should make this into a proper test with check files.

@nicolasstucki
Copy link
Contributor

Why didn't you add the tests/neg/module-class-name.check check file?

@odersky
Copy link
Contributor Author

odersky commented May 10, 2019

Why didn't you add the tests/neg/module-class-name.check check file?

I never can figure out how to generate that check file in the first place. Can you add it?

It would be easy if it was just copy output to check. Please, everyone, let's bring that back. I hate obscure sbt commands that only work in some contexts but not in others.

@nicolasstucki
Copy link
Contributor

Done.

To add those, add the check file and run the test with --update-checkfiles

$ touch tests/neg/module-class-name.check
$ sbt 
> testCompilation tests/neg/module-class-name.scala --update-checkfiles

testCompilation --help could be used to remember the command.

@anatoliykmetyuk could you have a look at how we could make the checkfile creation process simpler?

@anatoliykmetyuk
Copy link
Contributor

@nicolasstucki ok, I'll have a look at it.

@biboudis
Copy link
Contributor

And document it somewhere as well! 🙈

@nicolasstucki
Copy link
Contributor

testCompilation --help might be a good place to add extra documentation.

@anatoliykmetyuk
Copy link
Contributor

The issue with testCompilation is that sometimes you need testOnly.

@anatoliykmetyuk anatoliykmetyuk merged commit cdd844f into scala:master May 10, 2019
@anatoliykmetyuk anatoliykmetyuk deleted the change-explaining-printer branch May 10, 2019 12:59
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