Skip to content

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 8 commits into from
Jun 23, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions lib/elixir/test/elixir/behaviour_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,33 @@ Code.require_file "test_helper.exs", __DIR__
defmodule BehaviourTest do
use ExUnit.Case, async: true

defmodule Sample do
use Behaviour
{_, _, sample_binary, _} =
defmodule Sample do
use Behaviour

defcallback first(integer) :: integer
defcallback first(integer) :: integer

defcallback foo(atom(), binary) :: binary
defcallback foo(atom(), binary) :: binary

defcallback bar(External.hello, my_var :: binary) :: binary
defcallback bar(External.hello, my_var :: binary) :: binary

defcallback guarded(my_var) :: my_var when my_var: binary
defcallback guarded(my_var) :: my_var when my_var: binary

defcallback orr(atom | integer) :: atom
defcallback orr(atom | integer) :: atom

defcallback literal(123, {atom}, :atom, [integer], true) :: atom
defcallback literal(123, {atom}, :atom, [integer], true) :: atom

defmacrocallback last(integer) :: Macro.t
end
defmacrocallback last(integer) :: Macro.t
end

@sample_binary sample_binary

test "callbacks" do
assert Sample.__behaviour__(:callbacks) == [first: 1, guarded: 1, "MACRO-last": 2, literal: 5, orr: 1, foo: 2, bar: 2]
end

test "specs" do
assert length(Keyword.get_values(Sample.module_info[:attributes], :callback)) == 7
assert length(Kernel.Typespec.beam_callbacks(@sample_binary)) == 7
end

test "default is not supported" do
Expand Down
2 changes: 2 additions & 0 deletions lib/elixir/test/elixir/kernel/dialyzer_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ defmodule Kernel.DialyzerTest do
{:ok, [outdir: dir, dialyzer: dialyzer]}
end

@tag otp19: false
test "no warnings on valid remote calls", context do
copy_beam! context, Dialyzer.RemoteCall
assert_dialyze_no_warnings! context
Expand All @@ -63,6 +64,7 @@ defmodule Kernel.DialyzerTest do
assert_dialyze_no_warnings! context
end

@tag otp19: false
test "no warnings on raise", context do
copy_beam! context, Dialyzer.Raise
assert_dialyze_no_warnings! context
Expand Down
5 changes: 3 additions & 2 deletions lib/elixir/test/elixir/kernel/raise_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,10 @@ defmodule Kernel.RaiseTest do
end

test "badfun error" do
x = :example
# Avoid "invalid function call" warning in >= OTP 19
x = fn -> :example end
result = try do
x.(2)
x.().(2)
rescue
x in [BadFunctionError] -> Exception.message(x)
end
Expand Down
44 changes: 26 additions & 18 deletions lib/elixir/test/elixir/protocol_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,24 @@ defmodule ProtocolTest do

doctest Protocol

defprotocol Sample do
@type t :: any
@doc "Ok"
@spec ok(t) :: boolean
def ok(term)
end
{_, _, sample_binary, _} =
defprotocol Sample do
@type t :: any
@doc "Ok"
@spec ok(t) :: boolean
def ok(term)
end

defprotocol WithAny do
@fallback_to_any true
@doc "Ok"
def ok(term)
end
@sample_binary sample_binary

{_, _, with_any_binary, _} =
defprotocol WithAny do
@fallback_to_any true
@doc "Ok"
def ok(term)
end

@with_any_binary with_any_binary

defprotocol Derivable do
def ok(a)
Expand Down Expand Up @@ -123,11 +129,11 @@ defmodule ProtocolTest do
end

test "protocol defines callbacks" do
assert get_callbacks(Sample, :ok, 1) ==
[{:type, [11], :fun, [{:type, [11], :product, [{:user_type, [11], :t, []}]}, {:type, [11], :boolean, []}]}]
assert get_callbacks(@sample_binary, :ok, 1) ==
[{:type, 12, :fun, [{:type, 12, :product, [{:user_type, 12, :t, []}]}, {:type, 12, :boolean, []}]}]

assert get_callbacks(WithAny, :ok, 1) ==
[{:type, [18], :fun, [{:type, [18], :product, [{:user_type, [18], :t, []}]}, {:type, [18], :term, []}]}]
assert get_callbacks(@with_any_binary, :ok, 1) ==
[{:type, 22, :fun, [{:type, 22, :product, [{:user_type, 22, :t, []}]}, {:type, 22, :term, []}]}]
end

test "protocol defines functions and attributes" do
Expand Down Expand Up @@ -182,8 +188,8 @@ defmodule ProtocolTest do
assert Multi.test(:a) == :a
end

defp get_callbacks(module, name, arity) do
callbacks = for {:callback, info} <- module.__info__(:attributes), do: hd(info)
defp get_callbacks(beam, name, arity) do
callbacks = Kernel.Typespec.beam_callbacks(beam)
List.keyfind(callbacks, {name, arity}, 0) |> elem(1)
end

Expand Down Expand Up @@ -308,6 +314,8 @@ defmodule Protocol.ConsolidationTest do
{:ok, binary} = Protocol.consolidate(Sample, [Any, ImplStruct])
:code.load_binary(Sample, 'protocol_test.exs', binary)

@sample_binary binary

# Any should be moved to the end
:code.purge(WithAny)
:code.delete(WithAny)
Expand Down Expand Up @@ -367,7 +375,7 @@ defmodule Protocol.ConsolidationTest do
end

test "consolidated keeps callbacks" do
callbacks = for {:callback, info} <- Sample.__info__(:attributes), do: hd(info)
callbacks = Kernel.Typespec.beam_callbacks(@sample_binary)
assert callbacks != []
end

Expand Down
8 changes: 7 additions & 1 deletion lib/elixir/test/elixir/test_helper.exs
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
ExUnit.start [trace: "--trace" in System.argv]
exclude =
case :erlang.system_info(:otp_release) do
Copy link
Member

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:

otp_release = List.to_integer :erlang.system_info(:otp_release)
ExUnit.start [exclude: [:otp_release], include: [otp_release: otp_release], trace: "--trace" in System.argv]

And for the tests that do not work on 19, we can do: @tag otp_release: 18.

What do you think?

Copy link
Member

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?

Copy link
Contributor Author

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?

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Member

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.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, one step at a time. And this are tests, we can afford to be flexible and change later on.

OK lets stick with :otp_release I was just trying to drive home the point that this kind of version checking is often inadequate.

Copy link
Contributor Author

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?

Copy link
Member

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:

In lib/elixir/test/elixir/test_helper.exs
#4855 (comment):

@@ -1,4 +1,10 @@
-ExUnit.start [trace: "--trace" in System.argv]
+exclude =

  • case :erlang.system_info(:otp_release) do

Ok, so I'm gonna stick with it as is so that the tests start running again
on OTP 20? Does that sound ok?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/elixir-lang/elixir/pull/4855/files/1bd0b165454dd143d26c34444bdb599faa942b2a#r68151089,
or mute the thread
https://github.com/notifications/unsubscribe/AAAlbjt2iwzcU19WfKbQVrDCCB6H1CI4ks5qOb-3gaJpZM4I8OnO
.

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

Copy link
Contributor Author

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. :)

'19' -> [otp19: false]
_ -> []
end

ExUnit.start [exclude: exclude, trace: "--trace" in System.argv]

# Beam files compiled on demand
path = Path.expand("../../tmp/beams", __DIR__)
Expand Down
9 changes: 3 additions & 6 deletions lib/iex/lib/iex/introspection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ defmodule IEx.Introspection do
if docs = Code.get_docs(mod, :docs) do
if doc = find_doc(docs, fun, arity) do
if callback_module = is_nil(elem(doc, 4)) and callback_module(mod, fun, arity) do
filter = &match?({^fun, _}, elem(&1, 0))
filter = &match?({^fun, ^arity}, elem(&1, 0))
print_callback_docs(callback_module, filter, &print_doc/2)
else
print_doc(doc)
Expand Down Expand Up @@ -147,14 +147,11 @@ defmodule IEx.Introspection do
do: true

defp callback_module(mod, fun, arity) do
filter = &match?({{^fun, ^arity}, _}, &1)
mod.module_info(:attributes)
|> Keyword.get_values(:behaviour)
|> Stream.concat()
|> Enum.find(fn module ->
module.module_info(:attributes)
|> Enum.filter(&match?({:callback, _}, &1))
|> Enum.any?(&match?({_, [{{^fun, ^arity}, _} | _]}, &1))
end)
|> Enum.find(&Enum.any?(Typespec.beam_callbacks(&1), filter))
end

defp print_doc({{fun, _}, _line, kind, args, doc}) do
Expand Down
10 changes: 7 additions & 3 deletions lib/iex/test/iex/helpers_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -77,25 +77,29 @@ defmodule IEx.HelpersTest do
@doc "Docs for MyBehaviour.first"
@callback first(integer) :: integer
@callback second(integer) :: integer
@callback second(integer, integer) :: integer
end
"""
impl = """
defmodule Impl do
@behaviour MyBehaviour
def first(0), do: 0
@doc "Docs for Impl.second"
@doc "Docs for Impl.second/1"
def second(0), do: 0
@doc "Docs for Impl.second/2"
def second(0, 0), do: 0
end
"""
files = ["my_behaviour.ex", "impl.ex"]
with_file files, [behaviour, impl], fn ->
assert c(files, ".") |> Enum.sort == [Impl, MyBehaviour]

assert capture_io(fn -> h Impl.first/1 end) == "* @callback first(integer()) :: integer()\n\nDocs for MyBehaviour.first\n"
assert capture_io(fn -> h Impl.second/1 end) == "* def second(int)\n\nDocs for Impl.second\n"
assert capture_io(fn -> h Impl.second/1 end) == "* def second(int)\n\nDocs for Impl.second/1\n"
assert capture_io(fn -> h Impl.second/2 end) == "* def second(int1, int2)\n\nDocs for Impl.second/2\n"

assert capture_io(fn -> h Impl.first end) == "* @callback first(integer()) :: integer()\n\nDocs for MyBehaviour.first\n"
assert capture_io(fn -> h Impl.second end) == "* def second(int)\n\nDocs for Impl.second\n"
assert capture_io(fn -> h Impl.second end) == "* def second(int)\n\nDocs for Impl.second/1\n* def second(int1, int2)\n\nDocs for Impl.second/2\n"
end
after
cleanup_modules([Impl, MyBehaviour])
Expand Down
1 change: 0 additions & 1 deletion lib/iex/test/test_helper.exs
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,3 @@ defmodule IEx.Case do
|> String.trim
end
end