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

Delete dead, name-forcing code - fixing nightlies #13901

Closed
wants to merge 1 commit into from

Conversation

dwijnand
Copy link
Member

@dwijnand dwijnand commented Nov 7, 2021

[test_java8]

@dwijnand dwijnand linked an issue Nov 7, 2021 that may be closed by this pull request
@dwijnand
Copy link
Member Author

dwijnand commented Nov 8, 2021

This fixes CI but breaks locally running the test, at least for me.

assert(1234567890123456789L.round == 1234567890123456789L) // error
assert(math.round(1234567890123456789L) == 1234567890123456789L) // error
assert(123456789.round == 123456789) // error (Int to Flaot)
assert(math.round(123456789) == 123456789)
Copy link
Contributor

Choose a reason for hiding this comment

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

That behavior seems correct. round is overloaded, so there is no expected type yet. This seems to indicate that the tests are done in the wrong phase. Maybe we should test this only after Typer?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe test assert(math.round(123456789) == 123456789) in another test file to test the other error/warning.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the test was correct. I just wanted a pull request ready that we could merge today to unbreak the nightly builds. I spent some time with Lukas and I think we should be able to merge #13911 instead. If something comes up I'll PR moving the test instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ironically, @nicolasstucki, it's thanks to the fact that these were in the same file that we even found that the wrong warning was being emitted. I'm still not sure I like this new "// error" with no message assertion practice that scala 3 has introduced..

@dwijnand
Copy link
Member Author

dwijnand commented Nov 8, 2021

Duplicate of #13911

@dwijnand dwijnand marked this as a duplicate of #13911 Nov 8, 2021
@dwijnand dwijnand closed this Nov 8, 2021
@dwijnand dwijnand deleted the random-nightly-fix branch November 8, 2021 11:41
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.

Nightly Dotty workflow of 2021-11-04 failed
3 participants