Skip to content

Cleanup tests #5600

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
Mar 5, 2019
Merged

Cleanup tests #5600

merged 3 commits into from
Mar 5, 2019

Conversation

allanrenucci
Copy link
Contributor

  • Remove std lib blacklist
  • Factor out some code

@allanrenucci
Copy link
Contributor Author

Blocked on #5433

@odersky
Copy link
Contributor

odersky commented Dec 13, 2018

Why not wait for 2.13 collections instead? I mean the situation with 2.12 is both acceptable and temporary, so why bother changing it now? I agree that #5433 needs to be addressed.

@allanrenucci
Copy link
Contributor Author

Why not wait for 2.13 collections instead? I mean the situation with 2.12 is both acceptable and temporary, so why bother changing it now? I agree that #5433 needs to be addressed.

I am not sure to understand your comment. This PR is unrelated to #5433. It is a little refactoring in the test infrastructure. It just happen to change the compilation order in some tests which now fail

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

Nice simplification! While you're at it, could you try moving the sources in a different package than package scala ? We're only interested in testing that these files still compile and not in actually using them as the real standard library (since we'll switch to 2.13 at some point anyway), therefore using a different package avoids interaction between these files and the symbols the compiler load (in particular, it should allow us to completely get rid of the blacklist).

@allanrenucci
Copy link
Contributor Author

allanrenucci commented Mar 5, 2019

Nice simplification! While you're at it, could you try moving the sources in a different package than package scala ?

Can this be done in a separate PR. I probably won't have time to do that in the near future and I don't think it should block this PR

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

Can this be done in a separate PR. I probably won't have time to do that in the near feature and I don't think it should block this PR

Sure.

@allanrenucci allanrenucci merged commit 2a7edab into scala:master Mar 5, 2019
@allanrenucci allanrenucci deleted the test-cleanup branch March 5, 2019 19:14
@ghost ghost removed the stat:needs review label Mar 5, 2019
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.

3 participants