chore: several clean ups

- Removed test config options - updated deps - fixed README - Version bump - Travis proper scripting
victorolinasc · Nov 10, 2018 · 43ec373 · 43ec373
1 parent c4d3c9e
commit 43ec373
Show file tree

Hide file tree

Showing 11 changed files with 219 additions and 51 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,5 +1,5 @@
 language: elixir
-sudo: false # to use faster container based build environment
+sudo: false
 
 matrix:
   include:
@@ -11,10 +11,10 @@ matrix:
       elixir: 1.6.1
     - otp_release: 20.2
       elixir: 1.6.1
+    - otp_release: 21.1
+      elixir: 1.7.4
+
+script:
+  - mix credo --strict
+  - MIX_ENV=test mix coveralls.travis --trace --cover
 
-after_script:
-  - mix deps.get
-  - MIX_ENV=dev mix credo
-  - mix docs # generate docs to check if it is not breaking.
-  - MIX_ENV=test mix coveralls.travis 
-
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,17 @@
+## 2.3.0
+
+### Added:
+
+- Error messages now retain their whole format from standard ExUnit formatter (thanks to @hauleth)
+- Refactored the tests to use xpath (Thanks to @hauleth)
+- Refactored the code base to be more modern (thanks to @hauleth)
+
+### Bugfixes:
+
+- Fixed options description in README
+- Better CI integration: Credo, Check formatting, test tracing
+- Updated deps
+
 ## 2.2.0
 
 ### Bugfixes:

diff --git a/README.md b/README.md
@@ -7,9 +7,7 @@ A simple ExUnit Formatter that collects test results and generates an xml report
 
 The report is generated in `Mix.Project.app_path` folder with a default filename of test-junit-report.xml. It can be configured through application configuration on the key report_file (application junit_formatter).
 
-## OBSERVATION
-
-Versions 2+ require minimum Elixir version to be 1.4+. For older releases, please use version 1.3 of this library.
+> Versions 2+ require minimum Elixir version to be 1.4+. For older releases, please use version 1.3 of this library.
 
 ## Usage
 
@@ -18,7 +16,7 @@ First, add `JUnitFormatter` to the dependencies in your mix.exs:
 ```elixir
   defp deps do
     [
-      {:junit_formatter, "~> 2.2", only: [:test]}
+      {:junit_formatter, "~> 2.3", only: [:test]}
     ]
   end
 ```
@@ -68,9 +66,9 @@ The JUnit style XML report for this project looks like this:
 
 ## Options
 
-`JUnitFormatter` accepts 2 options that can be passed in config.exs (or equivalent environment configuration for tests):
+`JUnitFormatter` accepts 4 options that can be passed in config.exs (or equivalent environment configuration for tests):
 
-- `print_report_file` (boolean - default `false`): tells formatter if you want to see the path where the file is being written to in the console (`Logger.debug fn -> "Junit-formatter report at: #{report_path}" end`). This might help you debug where the file is. By default it writes the report to the `Mix.Project.app_path` folder. This ensures compatibility with umbrella apps.
+- `print_report_file` (boolean - default `false`): tells formatter if you want to see the path where the file is being written to in the console. This might help you debug where the file is. By default it writes the report to the `Mix.Project.app_path` folder. This ensures compatibility with umbrella apps.
 - `report_file` (binary - default `"test-junit-report.xml"`): the name of the file to write to. It must contain the extension. 99% of the time you will want the extension to be `.xml`, but if you don't you can pass any extension (though the contents of the file will be an xml document). 
 - `report_dir` (binary - default `Mix.Project.app_path()`): the directory to which the formatter will write the report. Do not end it with a slash. **IMPORTANT!!** `JUnitFormatter` will **NOT** create the directory. If you are pointing to a directory that is outside _build then it is your duty to clean it and to be sure it exists.
 - `prepend_project_name?` (boolean - default `false`): tells if the report file should have the name of the project as a prefix. See the "Umbrella" part of the documentation.

diff --git a/config/.credo.exs b/config/.credo.exs
@@ -0,0 +1,152 @@
+# This file contains the configuration for Credo and you are probably reading
+# this after creating it with `mix credo.gen.config`.
+#
+# If you find anything wrong or unclear in this file, please report an
+# issue on GitHub: https://github.com/rrrene/credo/issues
+#
+%{
+  #
+  # You can have as many configs as you like in the `configs:` field.
+  configs: [
+    %{
+      #
+      # Run any exec using `mix credo -C <name>`. If no exec name is given
+      # "default" is used.
+      #
+      name: "default",
+      #
+      # These are the files included in the analysis:
+      files: %{
+        #
+        # You can give explicit globs or simply directories.
+        # In the latter case `**/*.{ex,exs}` will be used.
+        #
+        included: ["lib/", "src/", "test/", "web/", "apps/"],
+        excluded: [~r"/_build/", ~r"/deps/", ~r"/node_modules/"]
+      },
+      #
+      # If you create your own checks, you must specify the source files for
+      # them here, so they can be loaded by Credo before running the analysis.
+      #
+      requires: [],
+      #
+      # If you want to enforce a style guide and need a more traditional linting
+      # experience, you can change `strict` to `true` below:
+      #
+      strict: false,
+      #
+      # If you want to use uncolored output by default, you can change `color`
+      # to `false` below:
+      #
+      color: true,
+      #
+      # You can customize the parameters of any check by adding a second element
+      # to the tuple.
+      #
+      # To disable a check put `false` as second element:
+      #
+      #     {Credo.Check.Design.DuplicatedCode, false}
+      #
+      checks: [
+        #
+        ## Consistency Checks
+        #
+        {Credo.Check.Consistency.ExceptionNames, []},
+        {Credo.Check.Consistency.LineEndings, []},
+        {Credo.Check.Consistency.ParameterPatternMatching, []},
+        {Credo.Check.Consistency.SpaceAroundOperators, []},
+        {Credo.Check.Consistency.SpaceInParentheses, []},
+        {Credo.Check.Consistency.TabsOrSpaces, []},
+
+        #
+        ## Design Checks
+        #
+        # You can customize the priority of any check
+        # Priority values are: `low, normal, high, higher`
+        #
+        {Credo.Check.Design.AliasUsage,
+         [priority: :low, if_nested_deeper_than: 2, if_called_more_often_than: 0]},
+        # You can also customize the exit_status of each check.
+        # If you don't want TODO comments to cause `mix credo` to fail, just
+        # set this value to 0 (zero).
+        #
+        {Credo.Check.Design.TagTODO, [exit_status: 2]},
+        {Credo.Check.Design.TagFIXME, []},
+
+        #
+        ## Readability Checks
+        #
+        {Credo.Check.Readability.AliasOrder, []},
+        {Credo.Check.Readability.FunctionNames, []},
+        {Credo.Check.Readability.LargeNumbers, []},
+        {Credo.Check.Readability.MaxLineLength, [priority: :low, max_length: 120]},
+        {Credo.Check.Readability.ModuleAttributeNames, []},
+        {Credo.Check.Readability.ModuleDoc, []},
+        {Credo.Check.Readability.ModuleNames, []},
+        {Credo.Check.Readability.ParenthesesInCondition, []},
+        {Credo.Check.Readability.ParenthesesOnZeroArityDefs, []},
+        {Credo.Check.Readability.PredicateFunctionNames, []},
+        {Credo.Check.Readability.PreferImplicitTry, []},
+        {Credo.Check.Readability.RedundantBlankLines, []},
+        {Credo.Check.Readability.Semicolons, []},
+        {Credo.Check.Readability.SpaceAfterCommas, []},
+        {Credo.Check.Readability.StringSigils, []},
+        {Credo.Check.Readability.TrailingBlankLine, []},
+        {Credo.Check.Readability.TrailingWhiteSpace, []},
+        {Credo.Check.Readability.VariableNames, []},
+
+        #
+        ## Refactoring Opportunities
+        #
+        {Credo.Check.Refactor.CondStatements, []},
+        {Credo.Check.Refactor.CyclomaticComplexity, []},
+        {Credo.Check.Refactor.FunctionArity, []},
+        {Credo.Check.Refactor.LongQuoteBlocks, []},
+        {Credo.Check.Refactor.MapInto, []},
+        {Credo.Check.Refactor.MatchInCondition, []},
+        {Credo.Check.Refactor.NegatedConditionsInUnless, []},
+        {Credo.Check.Refactor.NegatedConditionsWithElse, []},
+        {Credo.Check.Refactor.Nesting, []},
+        {Credo.Check.Refactor.PipeChainStart,
+         [excluded_argument_types: [:atom, :binary, :fn, :keyword], excluded_functions: []]},
+        {Credo.Check.Refactor.UnlessWithElse, []},
+
+        #
+        ## Warnings
+        #
+        {Credo.Check.Warning.BoolOperationOnSameValues, []},
+        {Credo.Check.Warning.ExpensiveEmptyEnumCheck, []},
+        {Credo.Check.Warning.IExPry, []},
+        {Credo.Check.Warning.IoInspect, []},
+        {Credo.Check.Warning.LazyLogging, []},
+        {Credo.Check.Warning.OperationOnSameValues, []},
+        {Credo.Check.Warning.OperationWithConstantResult, []},
+        {Credo.Check.Warning.RaiseInsideRescue, []},
+        {Credo.Check.Warning.UnusedEnumOperation, []},
+        {Credo.Check.Warning.UnusedFileOperation, []},
+        {Credo.Check.Warning.UnusedKeywordOperation, []},
+        {Credo.Check.Warning.UnusedListOperation, []},
+        {Credo.Check.Warning.UnusedPathOperation, []},
+        {Credo.Check.Warning.UnusedRegexOperation, []},
+        {Credo.Check.Warning.UnusedStringOperation, []},
+        {Credo.Check.Warning.UnusedTupleOperation, []},
+
+        #
+        # Controversial and experimental checks (opt-in, just remove `, false`)
+        #
+        {Credo.Check.Consistency.MultiAliasImportRequireUse, false},
+        {Credo.Check.Design.DuplicatedCode, false},
+        {Credo.Check.Readability.Specs, false},
+        {Credo.Check.Refactor.ABCSize, false},
+        {Credo.Check.Refactor.AppendSingleItem, false},
+        {Credo.Check.Refactor.DoubleBooleanNegation, false},
+        {Credo.Check.Refactor.VariableRebinding, false},
+        {Credo.Check.Warning.MapGetUnsafePass, false}
+
+        #
+        # Custom checks can be created using `mix credo.gen.check`.
+        #
+      ]
+    }
+  ]
+}
diff --git a/config/config.exs b/config/config.exs
@@ -1,5 +1,3 @@
 use Mix.Config
 
 config :junit_formatter, report_file: "report_file_test.xml"
-
-if Mix.env() == :test, do: import_config("#{Mix.env()}.exs")
diff --git a/config/test.exs b/config/test.exs
diff --git a/lib/formatter.ex b/lib/formatter.ex
@@ -53,8 +53,7 @@ defmodule JUnitFormatter do
 
   defstruct cases: %{}, properties: %{}
 
-  ## Formatter callbacks: may use opts in the future to configure file name pattern
-
+  @impl true
   def init(opts) do
     {:ok,
      %__MODULE__{
@@ -65,6 +64,7 @@ defmodule JUnitFormatter do
      }}
   end
 
+  @impl true
   def handle_cast({:suite_finished, _run_us, _load_us}, config) do
     # do the real magic
     suites = Enum.map(config.cases, &generate_testsuite_xml(&1, config.properties))
@@ -117,7 +117,7 @@ defmodule JUnitFormatter do
 
   @doc "Formats time from nanos to seconds"
   @spec format_time(integer) :: binary
-  def format_time(time), do: :io_lib.format('~.4f', [time / 1_000_000]) |> List.to_string()
+  def format_time(time), do: '~.4f' |> :io_lib.format([time / 1_000_000]) |> List.to_string()
 
   @doc """
   Helper function to get the full path of the generated report file.
@@ -140,17 +140,22 @@ defmodule JUnitFormatter do
 
   defp adjust_case_stats(%ExUnit.Test{case: name, time: time} = test, type, state) do
     cases =
-      Map.update(state.cases, name, struct(Stats, [{type, 1}, test_cases: [test], time: time, tests: 1]), fn stats ->
-        stats =
-          struct(
-            stats,
-            test_cases: [test | stats.test_cases],
-            time: stats.time + time,
-            tests: stats.tests + 1
-          )
-
-        if type, do: Map.update!(stats, type, &(&1 + 1)), else: stats
-      end)
+      Map.update(
+        state.cases,
+        name,
+        struct(Stats, [{type, 1}, test_cases: [test], time: time, tests: 1]),
+        fn stats ->
+          stats =
+            struct(
+              stats,
+              test_cases: [test | stats.test_cases],
+              time: stats.time + time,
+              tests: stats.tests + 1
+            )
+
+          if type, do: Map.update!(stats, type, &(&1 + 1)), else: stats
+        end
+      )
 
     %{state | cases: cases}
   end

diff --git a/mix.exs b/mix.exs
@@ -1,7 +1,7 @@
 defmodule Formatter.Mixfile do
   use Mix.Project
 
-  @version "2.2.0"
+  @version "2.3.0"
 
   def project do
     [
@@ -33,10 +33,10 @@ defmodule Formatter.Mixfile do
   defp deps do
     [
       {:earmark, "~> 1.2", only: :dev},
-      {:ex_doc, "~> 0.18", only: :dev, runtime: false},
+      {:ex_doc, "~> 0.19", only: :dev, runtime: false},
       {:excoveralls, "~> 0.8", only: :test},
       {:exjsx, "~> 4.0", only: :test, override: true},
-      {:credo, "~> 0.9.0-rc6", only: [:dev, :test]},
+      {:credo, "~> 1.0.0-rc1", only: [:dev, :test]},
       {:sweet_xml, "~> 0.6", only: :test}
     ]
   end

diff --git a/mix.lock b/mix.lock
@@ -1,18 +1,23 @@
 %{
   "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm"},
-  "certifi": {:hex, :certifi, "2.0.0", "a0c0e475107135f76b8c1d5bc7efb33cd3815cb3cf3dea7aefdd174dabead064", [:rebar3], [], "hexpm"},
-  "credo": {:hex, :credo, "0.9.0-rc6", "637bcf7b6f39f7293d1cd599e677d7ec813bcb74ca35fc3ed4a464564298e1b1", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:poison, ">= 0.0.0", [hex: :poison, repo: "hexpm", optional: false]}], "hexpm"},
-  "earmark": {:hex, :earmark, "1.2.4", "99b637c62a4d65a20a9fb674b8cffb8baa771c04605a80c911c4418c69b75439", [:mix], [], "hexpm"},
-  "ex_doc": {:hex, :ex_doc, "0.18.3", "f4b0e4a2ec6f333dccf761838a4b253d75e11f714b85ae271c9ae361367897b7", [:mix], [{:earmark, "~> 1.1", [hex: :earmark, repo: "hexpm", optional: false]}], "hexpm"},
-  "excoveralls": {:hex, :excoveralls, "0.8.0", "99d2691d3edf8612f128be3f9869c4d44b91c67cec92186ce49470ae7a7404cf", [:mix], [{:exjsx, ">= 3.0.0", [hex: :exjsx, repo: "hexpm", optional: false]}, {:hackney, ">= 0.12.0", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"},
+  "certifi": {:hex, :certifi, "2.4.2", "75424ff0f3baaccfd34b1214184b6ef616d89e420b258bb0a5ea7d7bc628f7f0", [:rebar3], [{:parse_trans, "~>3.3", [hex: :parse_trans, repo: "hexpm", optional: false]}], "hexpm"},
+  "credo": {:hex, :credo, "1.0.0-rc1", "31655e89925d3a75d27fd6a06aaffa853017cf0d9411645e3a56974db950669b", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm"},
+  "earmark": {:hex, :earmark, "1.2.6", "b6da42b3831458d3ecc57314dff3051b080b9b2be88c2e5aa41cd642a5b044ed", [:mix], [], "hexpm"},
+  "ex_doc": {:hex, :ex_doc, "0.19.1", "519bb9c19526ca51d326c060cb1778d4a9056b190086a8c6c115828eaccea6cf", [:mix], [{:earmark, "~> 1.1", [hex: :earmark, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.7", [hex: :makeup_elixir, repo: "hexpm", optional: false]}], "hexpm"},
+  "excoveralls": {:hex, :excoveralls, "0.10.2", "fb4abd5b8a1b9d52d35e1162e7e2ea8bfb84b47ae07c38d39aa8ce64be0b0794", [:mix], [{:hackney, "~> 1.13", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm"},
   "exjsx": {:hex, :exjsx, "4.0.0", "60548841e0212df401e38e63c0078ec57b33e7ea49b032c796ccad8cde794b5c", [:mix], [{:jsx, "~> 2.8.0", [hex: :jsx, repo: "hexpm", optional: false]}], "hexpm"},
-  "hackney": {:hex, :hackney, "1.10.1", "c38d0ca52ea80254936a32c45bb7eb414e7a96a521b4ce76d00a69753b157f21", [:rebar3], [{:certifi, "2.0.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "5.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "1.0.2", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.1", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm"},
-  "idna": {:hex, :idna, "5.1.0", "d72b4effeb324ad5da3cab1767cb16b17939004e789d8c0ad5b70f3cea20c89a", [:rebar3], [{:unicode_util_compat, "0.3.1", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm"},
+  "hackney": {:hex, :hackney, "1.14.3", "b5f6f5dcc4f1fba340762738759209e21914516df6be440d85772542d4a5e412", [:rebar3], [{:certifi, "2.4.2", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "6.0.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "1.0.2", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.4", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm"},
+  "idna": {:hex, :idna, "6.0.0", "689c46cbcdf3524c44d5f3dde8001f364cd7608a99556d8fbd8239a5798d4c10", [:rebar3], [{:unicode_util_compat, "0.4.1", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm"},
+  "jason": {:hex, :jason, "1.1.2", "b03dedea67a99223a2eaf9f1264ce37154564de899fd3d8b9a21b1a6fd64afe7", [:mix], [{:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm"},
   "jsx": {:hex, :jsx, "2.8.3", "a05252d381885240744d955fbe3cf810504eb2567164824e19303ea59eef62cf", [:mix, :rebar3], [], "hexpm"},
+  "makeup": {:hex, :makeup, "0.5.5", "9e08dfc45280c5684d771ad58159f718a7b5788596099bdfb0284597d368a882", [:mix], [{:nimble_parsec, "~> 0.4", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm"},
+  "makeup_elixir": {:hex, :makeup_elixir, "0.10.0", "0f09c2ddf352887a956d84f8f7e702111122ca32fbbc84c2f0569b8b65cbf7fa", [:mix], [{:makeup, "~> 0.5.5", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm"},
   "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm"},
   "mimerl": {:hex, :mimerl, "1.0.2", "993f9b0e084083405ed8252b99460c4f0563e41729ab42d9074fd5e52439be88", [:rebar3], [], "hexpm"},
+  "nimble_parsec": {:hex, :nimble_parsec, "0.4.0", "ee261bb53214943679422be70f1658fff573c5d0b0a1ecd0f18738944f818efe", [:mix], [], "hexpm"},
+  "parse_trans": {:hex, :parse_trans, "3.3.0", "09765507a3c7590a784615cfd421d101aec25098d50b89d7aa1d66646bc571c1", [:rebar3], [], "hexpm"},
   "poison": {:hex, :poison, "3.1.0", "d9eb636610e096f86f25d9a46f35a9facac35609a7591b3be3326e99a0484665", [:mix], [], "hexpm"},
-  "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.1", "28a4d65b7f59893bc2c7de786dec1e1555bd742d336043fe644ae956c3497fbe", [:make, :rebar], [], "hexpm"},
+  "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.4", "f0eafff810d2041e93f915ef59899c923f4568f4585904d010387ed74988e77b", [:make, :mix, :rebar3], [], "hexpm"},
   "sweet_xml": {:hex, :sweet_xml, "0.6.5", "dd9cde443212b505d1b5f9758feb2000e66a14d3c449f04c572f3048c66e6697", [:mix], [], "hexpm"},
-  "unicode_util_compat": {:hex, :unicode_util_compat, "0.3.1", "a1f612a7b512638634a603c8f401892afbf99b8ce93a45041f8aaca99cadb85e", [:rebar3], [], "hexpm"},
+  "unicode_util_compat": {:hex, :unicode_util_compat, "0.4.1", "d869e4c68901dd9531385bb0c8c40444ebf624e60b6962d95952775cac5e90cd", [:rebar3], [], "hexpm"},
 }
diff --git a/test/formatter_test.exs b/test/formatter_test.exs
@@ -19,7 +19,7 @@ defmodule FormatterTest do
   describe "properties" do
     test "contains seed" do
       defsuite do
-        test "it will fail", do: assert false
+        test "it will fail", do: assert(false)
       end
 
       output = run_and_capture_output(seed: 0)
@@ -30,7 +30,7 @@ defmodule FormatterTest do
 
     test "contains date" do
       defsuite do
-        test "it will fail", do: assert false
+        test "it will fail", do: assert(false)
       end
 
       output = run_and_capture_output(seed: 0)
@@ -42,7 +42,7 @@ defmodule FormatterTest do
   describe "testsuites" do
     test "tag is present as a root" do
       defsuite do
-        test "it will fail", do: assert false
+        test "it will fail", do: assert(false)
       end
 
       output = run_and_capture_output()
@@ -117,7 +117,7 @@ defmodule FormatterTest do
   describe "failure" do
     test "contains proper message" do
       defsuite do
-        test "fails", do: assert false
+        test "fails", do: assert(false)
       end
 
       output = run_and_capture_output()
@@ -127,6 +127,7 @@ defmodule FormatterTest do
         ~x{//testcase[@name="test fails"]/failure[@message="Expected truthy, got false"]}
       )
     end
+
     test "it counts raises as failures" do
       defsuite do
         test "raise", do: raise(ArgumentError)