-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Skip/fix tests on OTP 19 #4855
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
Skip/fix tests on OTP 19 #4855
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
f101058
Callback-related tests use beam AST to find callbacks
ericentin 87d449a
Fix invalid function call warning on OTP 19
ericentin c1ca7eb
Skip dialyzer-related tests on OTP 19
ericentin 1bd0b16
Skip callback-related IEx test on OTP 19
ericentin 1172371
Fix IEx h helper for OTP 19
ericentin d0bb9a6
Use Kernel.Typespec.beam_callbacks/1 instead of custom helper for tests
ericentin 5a05544
Improve IEx h helper test for multi-arity callbacks
ericentin dcc4fd1
Change otp19 exclusion tag name
ericentin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,4 +70,3 @@ defmodule IEx.Case do | |
|> String.trim | ||
end | ||
end | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can do this:
And for the tests that do not work on 19, we can do:
@tag otp_release: 18
.What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will we continue to skip the tests if this get patched before OTP 20?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems the functional difference would be that then tests will be disabled on versions before and after 18, vs. the current approach which only disables the tests on 19. I think we'd rather keep the tests running on previous versions (for anyone still developing on 17, unless that is no longer supported?) and ensure that the disabled tests run again on any new versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would have to manually remove the skip for 19.1 for instance, in any event.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We wont be able to compile on 17 anymore so dont worry about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For future cases like this I think we should stick to using application versions because people can build custom versions of OTP that don't match an OTP version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK lets stick with
:otp_release
I was just trying to drive home the point that this kind of version checking is often inadequate.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so I'm gonna stick with it as is so that the tests start running again on OTP 20? Does that sound ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really don't like the current tag name though. Can we use a Boolean flag
for :otp19 then? And we exclude based on true and false.
On Thursday, June 23, 2016, Eric Entin [email protected] wrote:
José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine. I'm not married to the name. :)