Test framework for IEx #1137

alco · 2013-05-29T10:06:07Z

This is WIP. Curious to know if I'm moving in the right direction.

TODO

write docs for IEx.Case
test other helpers

josevalim · 2013-05-29T13:24:17Z

@alco this looks great, a couple comments:

Can we drop the itest macro in favor of regular functions? We are doing a bunch of work inside the macro body and that is a no-no. For example, it means we can't for example store a regex in a variable and use it because it expects __r__ and __R__ exactly. Macros should be last resort only. :)
Please move the test framework to test/support/helpers.exs or test/test_helper.exs and call it IEx.Case. That would be a bit more inline with what we do in Mix for example.

alco · 2013-05-29T13:26:53Z

@mururu Is it feasible to allow capture_io to return output in bursts? For instance, if I want to test a bunch of expressions in one IEx session and one of them is an exception, I need to construct a big regex to check the output.

See this test. I'd like to be able to rewrite it like this:

itest "h helper",
  expected: [%r/{:module,M,<<(.+?)>>,{:hello_world,1}}\n/, 
             "# M\n\nModule M doc:ok\n",
             """* def hello_world()

             Hello world
             * def hello_world(arg)

             Hello world 2
             :ok
             """],
  input:    ["""
    defmodule M do
      @moduledoc "Module M doc"

      @doc "Hello world"
      def hello_world

      @doc "Hello world 2"
      def hello_world(arg)
    end
    """, "h(M)", "h(M.hello_world)"]

So for each item in the input list, we'll get one item in the output. This doesn't necessarily need to be handled by capture_io itself, it could use message passing, for example. Then I would be able to put separate inputs/outputs into lists in IEx.TestFramework.

josevalim · 2013-05-29T13:32:48Z

lib/iex/test/iex/interactive_test.exs

+  itest "h helper",
+    expected: %r/{:module,M,<<(.+?)>>,{:hello_world,1}}\n# M\n\nModule M doc:ok\n\* def hello_world\(\)\n\nHello world\n\* def hello_world\(arg\)\n\nHello world 2\n:ok/,
+    input:    """
+      defmodule M do


We should cleanup a module after we dynamically define it in a test. :) Or for this case, couldn't we just call h in an existing module? Maybe defined those in IEx.Test.Interactive itself.

Using docs on the test module is a great idea.

Does ExDoc skip modules in test directories?

alco · 2013-05-29T13:34:59Z

For example, it means we can't for example store a regex in a variable and use it because it expects r and R exactly.

@josevalim This is intentional. You can also use functions if you like. For example, those tests are equivalent:

  itest "color",
    opts:     [colors: [eval_result: "red"]],
    input:    "1 + 2",
    expected: "\e[31m3\e[0m"

  test "color redux" do
    assert (capture_iex("1 + 2", [colors: [eval_result: "red"]])) == "\e[31m3\e[0m"
  end

I find the first one more preferable because it gives you a framework to fill the values in. If you need to test something tricky, you can use capture_iex directly.

josevalim · 2013-05-29T13:36:05Z

lib/iex/test/iex/interactive_test.exs

+  use IEx.TestFramework
+
+  # All tests that `use IEx.TestFramework` should go into this module to avoid
+  # race conditions with :iex application.


We are not going to have race conditions if all the modules are sync.

When I create another module and use IEx.TestFramework there, I get the {:error,{:already_started,:iex}} error when running tests.

I will need to take a better look then... but the test cases were meant to run on isolation. Unless :application.stop is async (i.e. when it returns, the application may not have stopped completely yet, which I don't think is the case. :)

My bad, I've written def teardown_all do instead of teardown_all do.

josevalim · 2013-05-29T13:41:31Z

@alco I am still not sold why we need macros here. Compare:

  itest "color",
    opts:     [colors: [eval_result: "red"]],
    input:    "1 + 2",
    expected: "\e[31m3\e[0m"

To:

  test "color" do
    opts = [colors: [eval_result: "red"]]
    assert capture_iex("1 + 2", opts) == "\e[31m3\e[0m"
  end

There isn't really a benefit and everyone knows exactly how the second one works. There is no gotchas about when to use regular expressions, which expressions are supported by the macro, etc...

alco · 2013-05-29T14:14:17Z

@josevalim Here are the same tests rewritten using test -- https://github.com/elixir-lang/elixir/pull/1137/files#diff-4

Does it look good?

mururu · 2013-05-29T15:07:02Z

@alco It is possible to allow capture_io to return output each time it receives an output request(:get_chars, :get_line and :get_until). But I don't know it is useful because one function might send many output request. In case of the example, h(M.hello_world) sends more than one output requests so the outputs will be divided into more than you expect.

alco · 2013-05-29T15:52:10Z

@mururu Thanks for the feedback.

alco · 2013-05-29T18:26:55Z

I've moved things around and renamed the modules. If it's OK, I will only need to add docs and tests for the remaining helpers.

josevalim · 2013-05-29T23:44:20Z

@alco this is perfect! Before you start working on the rest, do you mind squashing those commits together so we have the whole foundation set up in one single commit? Easier to visualize latter.

alco · 2013-05-30T14:05:48Z

@josevalim Please take another look. Tests for l and r are currently failing. I need your help with explaining the behavior marked as FIXME in the code.

josevalim · 2013-05-30T14:35:14Z

lib/iex/test/iex/options_test.exs

@@ -0,0 +1,28 @@
+Code.require_file "../test_helper.exs", __DIR__
+
+defmodule IEx.Options.Test do


IEx.OptionsTest :)

alco · 2013-05-30T16:09:01Z

@josevalim I've rewritten the tests using capture_io when appropriate.

Please take a look at FIXMEs in the tests for l helper and r helper.

alco · 2013-05-30T16:14:40Z

There appears to be some kind race condition between the tests. Contrary to what Travis shows, this is what I get:

josevalim · 2013-05-31T19:22:18Z

We also need to test this one... we should just not forget... 6397ced

josevalim · 2013-05-31T19:22:48Z

Basically, to reproduce it, we just need to type something like "%B(%{bar})" in the shell and it should not fail.

alco · 2013-05-31T19:33:39Z

I've added tests for color, but the aren't working. Colors are working correctly in IEx, but after capture_io some escapes are lost.

alco · 2013-05-31T19:38:17Z

Dough, I see what's wrong: capture_io is not treated as terminal, so IO.ANSI.escape disables coloring.

alco · 2013-06-01T00:16:21Z

This is almost ready. Once the helpers issues are resolved, I'd like to squash some commits before merging.

josevalim · 2013-06-01T01:41:21Z

lib/iex/test/iex/helpers_test.exs

+    File.rm! "Elixir.Helpers_test_module.beam"
+
+    # FIXME: This errors out with "Module 'Elixir.Helpers_test_module' must be purged before loading"
+    #true = :code.delete Helpers_test_module


The problem is in this commented line. When you call the l helper, it purges the old version and then loads a new version. By loading a new version, the previous version is now marked as old. So when you get to :code.delete to make the current new version as old, you can't actually do it, because the old version slot is still taken, so you do need to explicitly purge once again before. By fixing this, everything else seems to work fine (except the r test that still fails).

josevalim · 2013-06-01T01:43:52Z

@alco a couple quick notes: the test fixtures uses non conventional names like Helpers_test_module and test-foo-bar. Maybe they should follow Elixir's conventions too? Also, it may be worthy creating a couple local functions to clean up the helper tests, like a purge(module) (to delete and purge) and another one called with_helpers_test_module(callback) that would wrap the writing to a file and removing the file.

alco · 2013-06-01T06:08:06Z

Thanks, I'll fix those.

What about this line? That System.cmd call would better go away.

josevalim · 2013-06-01T06:27:34Z

Use the parallel compiler or c helper and then purge and delete the module?

José Valim
www.plataformatec.com.br
Founder and Lead Developer
*

alco · 2013-06-02T07:19:40Z

the test fixtures uses non conventional names like Helpers_test_module and test-foo-bar

I renamed the module to Sample. What name should I use for the file?

alco · 2013-06-02T07:20:41Z

Use the parallel compiler or c helper and then purge and delete the module?

I'd like to not interfere with the modules loaded in runtime while testing module loading. Is it technically not possible to compile a module from Erlang without loading it?

josevalim · 2013-06-02T07:44:23Z

Give the file the name of the module, like sample.ex. About the r helper,
feel free to change the docs or the return value. The important is that we
properly test it from now on. :)

José Valim
www.plataformatec.com.br
Founder and Lead Developer
*

alco · 2013-06-03T10:06:42Z

@josevalim Please take another look.

josevalim · 2013-06-03T11:38:44Z

lib/iex/lib/iex/helpers.ex

@@ -257,6 +264,7 @@ defmodule IEx.Helpers do
  """
  def l(module) do
    :code.purge(module)
+    # FIXME: shouldn't we use Code.load_file here?


No. They do different things. We want to continue using :code.load_file here.

josevalim · 2013-06-03T11:41:54Z

@alco it looks good! we should just get rid of the #FIXME that have been answered (we should use :code.load_file and there isn't a way of compiling a module without loading it) and then pls go ahead and merge it.

Test framework for IEx

josevalim reviewed May 29, 2013
View reviewed changes

josevalim mentioned this pull request May 30, 2013

Test framework for IEx #919

Closed

josevalim reviewed May 30, 2013
View reviewed changes

josevalim reviewed Jun 1, 2013
View reviewed changes

josevalim reviewed Jun 3, 2013
View reviewed changes

alco added 13 commits June 3, 2013 15:07

Implementation of a basic test framework for IEx. Closes elixir-lang#919

Verified

This commit was signed with the committer’s verified signature.

azat-io Azat S.

GPG key ID: D04DB9ECD3B3B4FC

Verified
Learn about vigilant mode

997a798

Write tests for IEx helpers

958550c

Rewrite tests for helpers to use capture_iex and capture_io sparingly

a79d965

Update the tests for l and r helpers

7951f73

Don't process ANSI escapes in ExUnit output

7f9a2ed

Add more tests for shell coloring

ecd3c19

Add docs for IEx test framework

a8d2e45

Clean up test code for module helpers

893a52a

Update the r helper

99c7d84

Update the docs for Code.load_file and Code.require_file

5e64033

Add prompt test

ed0b161

Proof-read the doc for capture_io

d636dc0

Minor comment cleanup

d26e4b3

alco added a commit that referenced this pull request Jun 3, 2013

Merge pull request #1137 from alco/919-iex-test-fmwk

75a1373

Test framework for IEx

alco merged commit 75a1373 into elixir-lang:master Jun 3, 2013

alco deleted the 919-iex-test-fmwk branch May 16, 2014 22:22

		@@ -0,0 +1,28 @@
		Code.require_file "../test_helper.exs", __DIR__

		defmodule IEx.Options.Test do

Test framework for IEx #1137

Test framework for IEx #1137

Conversation

alco commented May 29, 2013

josevalim commented May 29, 2013

alco commented May 29, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alco commented May 29, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josevalim commented May 29, 2013

alco commented May 29, 2013

mururu commented May 29, 2013

alco commented May 29, 2013

alco commented May 29, 2013

josevalim commented May 29, 2013

alco commented May 30, 2013

Choose a reason for hiding this comment

alco commented May 30, 2013

alco commented May 30, 2013

josevalim commented May 31, 2013

josevalim commented May 31, 2013

alco commented May 31, 2013

alco commented May 31, 2013

alco commented Jun 1, 2013

Choose a reason for hiding this comment

josevalim commented Jun 1, 2013

alco commented Jun 1, 2013

josevalim commented Jun 1, 2013

alco commented Jun 2, 2013

alco commented Jun 2, 2013

josevalim commented Jun 2, 2013

alco commented Jun 3, 2013

Choose a reason for hiding this comment

josevalim commented Jun 3, 2013