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

Don't run doctests for generated functions #1966

Merged
merged 9 commits into from
Jun 13, 2023
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
2 changes: 1 addition & 1 deletion .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ on:
- "v*.*.*"
env:
otp: "25.3.2"
elixir: "1.15.0-rc.1"
elixir: "1.15.0-rc.2"
jobs:
assets:
outputs:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ on:
- main
env:
otp: "25.3.2"
elixir: "1.15.0-rc.1"
elixir: "1.15.0-rc.2"
jobs:
main:
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/uffizzi-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ on:

env:
otp: "25.3.2"
elixir: "1.15.0-rc.1"
elixir: "1.15.0-rc.2"

jobs:
build-application:
Expand Down
3 changes: 1 addition & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# TODO: update the image once available on hexpm/ builds
ARG BASE_IMAGE=jonatanklosko/elixir:1.15.0-rc.1-erlang-25.3.2-debian-bullseye-20230522-slim
ARG BASE_IMAGE=hexpm/elixir:1.15.0-rc.2-erlang-25.3.2-debian-bullseye-20230522-slim

# Stage 1
# Builds the Livebook release
Expand Down
2 changes: 1 addition & 1 deletion docker/build_and_push.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
set -ex
cd "$(dirname "$0")/.."

elixir="1.15.0-rc.1"
elixir="1.15.0-rc.2"
erlang="25.3.2"
ubuntu="focal-20230126"

Expand Down
12 changes: 7 additions & 5 deletions lib/livebook/intellisense.ex
Original file line number Diff line number Diff line change
Expand Up @@ -515,12 +515,14 @@ defmodule Livebook.Intellisense do
name -> name
end

# TODO: remove the first check on Elixir v1.15.0
is_otp? =
case :code.which(app || module) do
:preloaded -> true
[_ | _] = path -> List.starts_with?(path, :code.lib_dir())
_ -> false
end
app == :erts or
Copy link
Member Author

@jonatanklosko jonatanklosko Jun 12, 2023

Choose a reason for hiding this comment

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

@josevalim in iex

Application.get_application(:atomics)
#=> nil

in iex -S mix

Application.get_application(:atomics)
# => :erts

Given the test fails, I think this changed in elixir-lang/elixir#12642, but probably expected?

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed!

Copy link
Member Author

Choose a reason for hiding this comment

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

Perfect, thanks! I commented out the failing test and left a TODO, so we can marge the fix :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I will leave the check and have the TODO to remove it, this way we keep the test.

case :code.which(app || module) do
:preloaded -> true
[_ | _] = path -> List.starts_with?(path, :code.lib_dir())
_ -> false
end

cond do
is_otp? ->
Expand Down
17 changes: 0 additions & 17 deletions lib/livebook/intellisense/docs.ex
Original file line number Diff line number Diff line change
Expand Up @@ -143,21 +143,4 @@ defmodule Livebook.Intellisense.Docs do
# loads elixir.beam, so we explicitly list it.
defp ensure_loaded?(Elixir), do: false
defp ensure_loaded?(module), do: Code.ensure_loaded?(module)

@doc """
Checks if the module has any documentation.
"""
@spec any_docs?(module()) :: boolean()
def any_docs?(module) do
case Code.fetch_docs(module) do
{:docs_v1, _, _, _, %{}, _, _} ->
true

{:docs_v1, _, _, _, _, _, docs} ->
Enum.any?(docs, &match?({_, _, _, %{}, _}, &1))

_ ->
false
end
end
end
4 changes: 1 addition & 3 deletions lib/livebook/runtime/evaluator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -460,9 +460,7 @@ defmodule Livebook.Runtime.Evaluator do
end

if ebin_path() do
new_context.env.context_modules
|> Enum.filter(&Livebook.Intellisense.Docs.any_docs?/1)
|> Livebook.Runtime.Evaluator.Doctests.run(code)
Livebook.Runtime.Evaluator.Doctests.run(new_context.env.context_modules, code)
end

state = put_context(state, ref, new_context)
Expand Down
63 changes: 52 additions & 11 deletions lib/livebook/runtime/evaluator/doctests.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,45 @@ defmodule Livebook.Runtime.Evaluator.Doctests do
Runs doctests in the given modules.
"""
@spec run(list(module()), String.t()) :: :ok
def run(modules, code)
def run(modules, code) do
doctests_specs =
for module <- modules, doctests_spec = doctests_spec(module), do: doctests_spec

def run([], _code), do: :ok
do_run(doctests_specs, code)
end

def run(modules, code) do
case define_test_module(modules) do
defp doctests_spec(module) do
case Code.fetch_docs(module) do
{:docs_v1, _, _, _, doc_content, _, member_docs} ->
funs =
for {{:function, name, arity}, annotation, _signatures, _doc, _meta} <- member_docs,
do: %{name: name, arity: arity, generated: :erl_anno.generated(annotation)}

{generated_funs, regular_funs} = Enum.split_with(funs, & &1.generated)

if regular_funs != [] or is_map(doc_content) do
except = Enum.map(generated_funs, &{&1.name, &1.arity})
%{module: module, except: except}
end

_ ->
nil
end
end

defp do_run([], _code), do: :ok

defp do_run(doctests_specs, code) do
case define_test_module(doctests_specs) do
{:ok, test_module} ->
if test_module.tests != [] do
lines = String.split(code, ["\r\n", "\n"])
lines = String.split(code, ["\r\n", "\n"])

test_module.tests
# Ignore test cases that don't actually point to a doctest
# in the source code
tests = Enum.filter(test_module.tests, &doctest_at_line?(lines, &1.tags.doctest_line))

if tests != [] do
tests
|> Enum.sort_by(& &1.tags.doctest_line)
|> Enum.each(fn test ->
report_doctest_running(test)
Expand All @@ -38,6 +66,18 @@ defmodule Livebook.Runtime.Evaluator.Doctests do
:ok
end

defp doctest_at_line?(lines, line_number) do
if line = Enum.at(lines, line_number - 1) do
case String.trim_leading(line) do
"iex>" <> _ -> true
"iex(" <> _ -> true
_ -> false
end
else
false
end
end

defp report_doctest_running(test) do
send_doctest_report(%{
line: test.tags.doctest_line,
Expand Down Expand Up @@ -122,9 +162,10 @@ defmodule Livebook.Runtime.Evaluator.Doctests do
end
end

defp define_test_module(modules) do
defp define_test_module(doctests_specs) do
id =
modules
doctests_specs
|> Enum.map(& &1.module)
|> Enum.sort()
|> Enum.map_join("-", fn module ->
module
Expand All @@ -140,8 +181,8 @@ defmodule Livebook.Runtime.Evaluator.Doctests do
defmodule name do
use ExUnit.Case, register: false

for module <- modules do
doctest module
for doctests_spec <- doctests_specs do
doctest doctests_spec.module, except: doctests_spec.except
end
end

Expand Down
2 changes: 1 addition & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ defmodule Livebook.MixProject do
@version "0.9.2"
@description "Automate code & data workflows with interactive notebooks"

@app_elixir_version "1.15.0-rc.1"
@app_elixir_version "1.15.0-rc.2"
@app_rebar3_version "3.22.0"

def project do
Expand Down
13 changes: 0 additions & 13 deletions test/livebook/intellisense/docs_test.exs

This file was deleted.

69 changes: 65 additions & 4 deletions test/livebook/runtime/evaluator_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ defmodule Livebook.Runtime.EvaluatorTest do
assert_receive {:runtime_evaluation_response, :code_1, {:error, message, :other},
metadata()}

assert clean_message(message) == """
assert """
** (FunctionClauseError) no function clause matching in List.first/2

The following arguments were given to List.first/2:
Expand All @@ -170,9 +170,7 @@ defmodule Livebook.Runtime.EvaluatorTest do
def first([], default)
def first([head | _], _default)

(elixir 1.15.0-rc.1) lib/list.ex:293: List.first/2
file.ex:1: (file)
"""
""" <> _ = clean_message(message)
end

test "returns additional metadata when there is a syntax error", %{evaluator: evaluator} do
Expand Down Expand Up @@ -630,6 +628,69 @@ defmodule Livebook.Runtime.EvaluatorTest do
status: :failed
}}
end

test "does not run generated doctests", %{evaluator: evaluator} do
code = ~S'''
defmodule Livebook.Runtime.EvaluatorTest.DoctestsGeneratedBase do
defmacro __using__(_) do
quote do
@doc """

iex> 1
2

iex> 2
2

"""
def foo, do: :ok
end
end
end

defmodule Livebook.Runtime.EvaluatorTest.DoctestsGenerated do
use Livebook.Runtime.EvaluatorTest.DoctestsGeneratedBase
end
'''

Evaluator.evaluate_code(evaluator, :elixir, code, :code_1, [])

assert_receive {:runtime_evaluation_response, :code_1, _, metadata()}
refute_received {:runtime_doctest_report, :code_1, %{}}

# Here the generated doctest line matches another iex> prompt
# in the module, but we expect the :erl_anno check to filter
# it out

code = ~S'''
defmodule Livebook.Runtime.EvaluatorTest.DoctestsGeneratedBase do
defmacro __using__(_) do
quote do
@doc """

iex> 1
2

"""
def foo, do: :ok
end
end
end

defmodule Livebook.Runtime.EvaluatorTest.DoctestsGenerated do
use Livebook.Runtime.EvaluatorTest.DoctestsGeneratedBase
@string """
iex> 1
2
"""
end
'''

Evaluator.evaluate_code(evaluator, :elixir, code, :code_1, [])

assert_receive {:runtime_evaluation_response, :code_1, _, metadata()}
refute_received {:runtime_doctest_report, :code_1, %{}}
end
end

describe "evaluate_code/6 identifier tracking" do
Expand Down
20 changes: 0 additions & 20 deletions test/support/test_modules/docs.ex

This file was deleted.