From 01d6f38ef1f7c9a6b49a44e92a8f14a8e5d7bc64 Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Thu, 8 Jul 2021 22:06:31 +0200 Subject: [PATCH 01/16] Introduce comment struct --- lib/elixir_analyzer.ex | 3 +- lib/elixir_analyzer/comment.ex | 11 ++++ lib/elixir_analyzer/constants.ex | 5 -- lib/elixir_analyzer/exercise_test.ex | 17 +++--- .../exercise_test/assert_call.ex | 2 +- .../exercise_test/assert_call/compiler.ex | 3 +- lib/elixir_analyzer/exercise_test/feature.ex | 2 +- .../exercise_test/feature/compiler.ex | 3 +- lib/elixir_analyzer/submission.ex | 21 ++++--- test/elixir_analyzer_test.exs | 12 ++++ .../missing_file_solution/.meta/config.json | 10 ++++ .../two_fer/missing_file_solution/README.md | 55 +++++++++++++++++++ .../missing_file_solution/test/test.exs | 10 ++++ .../test/two_fer_test.exs | 33 +++++++++++ 14 files changed, 158 insertions(+), 29 deletions(-) create mode 100644 lib/elixir_analyzer/comment.ex create mode 100644 test_data/two_fer/missing_file_solution/.meta/config.json create mode 100644 test_data/two_fer/missing_file_solution/README.md create mode 100644 test_data/two_fer/missing_file_solution/test/test.exs create mode 100644 test_data/two_fer/missing_file_solution/test/two_fer_test.exs diff --git a/lib/elixir_analyzer.ex b/lib/elixir_analyzer.ex index e572a443..e3ac545c 100644 --- a/lib/elixir_analyzer.ex +++ b/lib/elixir_analyzer.ex @@ -8,6 +8,7 @@ defmodule ElixirAnalyzer do alias ElixirAnalyzer.Constants alias ElixirAnalyzer.Submission + alias ElixirAnalyzer.Comment import ElixirAnalyzer.Summary, only: [summary: 2] @@ -204,7 +205,7 @@ defmodule ElixirAnalyzer do submission |> Submission.halt() - |> Submission.append_comment(%{ + |> Submission.append_comment(%Comment{ comment: Constants.general_file_not_found(), params: %{ "file_name" => submission.code_file, diff --git a/lib/elixir_analyzer/comment.ex b/lib/elixir_analyzer/comment.ex new file mode 100644 index 00000000..5226fd7f --- /dev/null +++ b/lib/elixir_analyzer/comment.ex @@ -0,0 +1,11 @@ +defmodule ElixirAnalyzer.Comment do + defstruct status: :test, name: nil, comment: nil, type: nil, suppress_if: nil, params: nil + + @type t :: %__MODULE__{ + name: String.t(), + comment: String.t(), + type: :essential | :actionable | :informative | :celebratory, + status: :skip | :test, + suppress_if: false | {String.t(), :pass | :fail} + } +end diff --git a/lib/elixir_analyzer/constants.ex b/lib/elixir_analyzer/constants.ex index 0605d103..f6ea94e3 100644 --- a/lib/elixir_analyzer/constants.ex +++ b/lib/elixir_analyzer/constants.ex @@ -10,11 +10,6 @@ defmodule ElixirAnalyzer.Constants do """ @constants [ - # Status Comments - # status_approve: "elixir.status.approve", - # status_disapprove: "elixir.status.disapprove", - # status_refer_to_mentor: "elixir.status.refer_to_mentor", - # General Error Comments general_file_not_found: "elixir.general.file_not_found", general_parsing_error: "elixir.general.parsing_error", diff --git a/lib/elixir_analyzer/exercise_test.ex b/lib/elixir_analyzer/exercise_test.ex index 07b2e3e6..07a53506 100644 --- a/lib/elixir_analyzer/exercise_test.ex +++ b/lib/elixir_analyzer/exercise_test.ex @@ -6,6 +6,7 @@ defmodule ElixirAnalyzer.ExerciseTest do alias ElixirAnalyzer.Submission alias ElixirAnalyzer.Constants + alias ElixirAnalyzer.Comment @doc false defmacro __using__(_opts) do @@ -66,7 +67,7 @@ defmodule ElixirAnalyzer.ExerciseTest do end defp any_result_matches_suppress_condition?(feature_results, condition) do - [suppress_on_test_name, suppress_on_result] = condition + {suppress_on_test_name, suppress_on_result} = condition Enum.any?(feature_results, fn {result, test} -> case {result, test.name} do @@ -81,16 +82,16 @@ defmodule ElixirAnalyzer.ExerciseTest do {:skip, _description}, submission -> submission - {:pass, description}, submission -> - if Map.get(description, :type, false) == :celebratory do - Submission.append_comment(submission, description) + {:pass, %Comment{} = comment}, submission -> + if Map.get(comment, :type, false) == :celebratory do + Submission.append_comment(submission, comment) else submission end - {:fail, description}, submission -> - if Map.get(description, :type, false) != :celebratory do - Submission.append_comment(submission, description) + {:fail, %Comment{} = comment}, submission -> + if Map.get(comment, :type, false) != :celebratory do + Submission.append_comment(submission, comment) else submission end @@ -106,7 +107,7 @@ defmodule ElixirAnalyzer.ExerciseTest do submission |> Submission.halt() - |> Submission.append_comment(%{ + |> Submission.append_comment(%Comment{ comment: Constants.general_parsing_error(), params: comment_params, type: :essential diff --git a/lib/elixir_analyzer/exercise_test/assert_call.ex b/lib/elixir_analyzer/exercise_test/assert_call.ex index eed0ff5f..c677086b 100644 --- a/lib/elixir_analyzer/exercise_test/assert_call.ex +++ b/lib/elixir_analyzer/exercise_test/assert_call.ex @@ -94,7 +94,7 @@ defmodule ElixirAnalyzer.ExerciseTest.AssertCall do end defp do_walk_assert_call_block({:suppress_if, _, [name, condition]} = node, test_data) do - {node, Map.put(test_data, :suppress_if, [name, condition])} + {node, Map.put(test_data, :suppress_if, {name, condition})} end defp do_walk_assert_call_block(node, test_data) do diff --git a/lib/elixir_analyzer/exercise_test/assert_call/compiler.ex b/lib/elixir_analyzer/exercise_test/assert_call/compiler.ex index a4269b48..7aabd672 100644 --- a/lib/elixir_analyzer/exercise_test/assert_call/compiler.ex +++ b/lib/elixir_analyzer/exercise_test/assert_call/compiler.ex @@ -7,6 +7,7 @@ defmodule ElixirAnalyzer.ExerciseTest.AssertCall.Compiler do """ alias ElixirAnalyzer.ExerciseTest.AssertCall + alias ElixirAnalyzer.Comment def compile(assert_call_data, code_ast) do name = assert_call_data.description @@ -18,7 +19,7 @@ defmodule ElixirAnalyzer.ExerciseTest.AssertCall.Compiler do suppress_if = Map.get(assert_call_data, :suppress_if, false) test_description = - Macro.escape(%{ + Macro.escape(%Comment{ name: name, comment: comment, type: type, diff --git a/lib/elixir_analyzer/exercise_test/feature.ex b/lib/elixir_analyzer/exercise_test/feature.ex index 1e0ee463..2995de7a 100644 --- a/lib/elixir_analyzer/exercise_test/feature.ex +++ b/lib/elixir_analyzer/exercise_test/feature.ex @@ -61,7 +61,7 @@ defmodule ElixirAnalyzer.ExerciseTest.Feature do end defp gather_feature_data({:suppress_if, _, [name, condition]} = node, acc) do - {node, put_in(acc, [:suppress_if], [name, condition])} + {node, put_in(acc, [:suppress_if], {name, condition})} end defp gather_feature_data({:depth, _, [f]} = node, acc) when is_integer(f) do diff --git a/lib/elixir_analyzer/exercise_test/feature/compiler.ex b/lib/elixir_analyzer/exercise_test/feature/compiler.ex index 1d7f8dd6..6a02c0c3 100644 --- a/lib/elixir_analyzer/exercise_test/feature/compiler.ex +++ b/lib/elixir_analyzer/exercise_test/feature/compiler.ex @@ -2,6 +2,7 @@ defmodule ElixirAnalyzer.ExerciseTest.Feature.Compiler do @moduledoc false alias ElixirAnalyzer.QuoteUtil + alias ElixirAnalyzer.Comment def compile({feature_data, feature_forms}, code_ast) do name = Keyword.fetch!(feature_data, :name) @@ -19,7 +20,7 @@ defmodule ElixirAnalyzer.ExerciseTest.Feature.Compiler do |> handle_combined_compiled_forms(find_type) test_description = - Macro.escape(%{ + Macro.escape(%Comment{ name: name, comment: comment, status: status, diff --git a/lib/elixir_analyzer/submission.ex b/lib/elixir_analyzer/submission.ex index f9951c52..82573b39 100644 --- a/lib/elixir_analyzer/submission.ex +++ b/lib/elixir_analyzer/submission.ex @@ -5,7 +5,7 @@ defmodule ElixirAnalyzer.Submission do JSON format { - "status": "...", + "summary": "...", "comments": [ { "comment": "elixir.general.some_paramaterised_message", @@ -17,14 +17,10 @@ defmodule ElixirAnalyzer.Submission do "elixir.general.some_paramaterised_message" ] } - The following statuses are valid: - - skipped: Something caused the analysis to be skipped - approve: To be used when a solution meets critera of a passing solution, comments MAY BE provided for improvement towards optimal. - disapprove: To be used when a solution can be disapproved as suboptimal and an actionable comment MUST BE provided. - refer_to_mentor: default status, a comment MAY BE provided. """ + alias ElixirAnalyzer.Comment + @enforce_keys [:code_file, :code_path, :path, :analysis_module] defstruct halted: false, halt_reason: nil, @@ -40,7 +36,7 @@ defmodule ElixirAnalyzer.Submission do halted: boolean, halt_reason: String.t() | nil, analyzed: boolean, - comments: list([binary | map]), + comments: list([Comment.t()]), path: String.t(), code_path: String.t(), code_file: String.t(), @@ -67,10 +63,13 @@ defmodule ElixirAnalyzer.Submission do end @doc false - def append_comment(%__MODULE__{} = submission, meta) when is_map(meta) do + def append_comment(%__MODULE__{} = submission, %Comment{} = comment) do comment = - Enum.filter(meta, fn - {key, _} when key in [:comment, :type, :params] -> true + comment + |> Map.from_struct() + |> Enum.filter(fn + {key, _} when key in [:comment, :type] -> true + {key, value} when key in [:params] and value != nil -> true _ -> false end) |> Enum.into(%{}) diff --git a/test/elixir_analyzer_test.exs b/test/elixir_analyzer_test.exs index 6b624cf7..d8012c74 100644 --- a/test/elixir_analyzer_test.exs +++ b/test/elixir_analyzer_test.exs @@ -46,6 +46,18 @@ defmodule ElixirAnalyzerTest do assert Submission.to_json(analyzed_exercise) == String.trim(expected_output) end + test "missing file solution" do + exercise = "two-fer" + path = "./test_data/two_fer/missing_file_solution/" + analyzed_exercise = ElixirAnalyzer.analyze_exercise(exercise, path, path, @options) + + expected_output = """ + {\"comments\":[{\"comment\":\"elixir.general.file_not_found\",\"params\":{\"file_name\":\"two_fer.ex\",\"path\":\"./test_data/two_fer/missing_file_solution/\"},\"type\":\"essential\"}],\"summary\":\"Check the comments for things to fix. 🛠\"} + """ + + assert Submission.to_json(analyzed_exercise) == String.trim(expected_output) + end + test "solution for an exercise with no analyzer module" do exercise = "not-a-real-exercise" path = "./test_data/two_fer/error_solution/" diff --git a/test_data/two_fer/missing_file_solution/.meta/config.json b/test_data/two_fer/missing_file_solution/.meta/config.json new file mode 100644 index 00000000..ed319ba5 --- /dev/null +++ b/test_data/two_fer/missing_file_solution/.meta/config.json @@ -0,0 +1,10 @@ +{ + "blurb": "Create a sentence of the form \"One for X, one for me.\"", + "authors": [], + "files": { + "solution": ["lib/two_fer.ex"], + "test": ["test/two_fer_test.exs"], + "example": [".meta/example.ex"] + }, + "source_url": "https://github.com/exercism/problem-specifications/issues/757" +} diff --git a/test_data/two_fer/missing_file_solution/README.md b/test_data/two_fer/missing_file_solution/README.md new file mode 100644 index 00000000..ce9d6b19 --- /dev/null +++ b/test_data/two_fer/missing_file_solution/README.md @@ -0,0 +1,55 @@ +# Two Fer + +`Two-fer` or `2-fer` is short for two for one. One for you and one for me. + +```text +"One for X, one for me." +``` + +When X is a name or "you". + +If the given name is "Alice", the result should be "One for Alice, one for me." +If no name is given, the result should be "One for you, one for me." + + +## Running tests + +Execute the tests with: + +```bash +$ elixir two_fer_test.exs +``` + +### Pending tests + +In the test suites, all but the first test have been skipped. + +Once you get a test passing, you can unskip the next one by +commenting out the relevant `@tag :pending` with a `#` symbol. + +For example: + +```elixir +# @tag :pending +test "shouting" do + assert Bob.hey("WATCH OUT!") == "Whoa, chill out!" +end +``` + +Or, you can enable all the tests by commenting out the +`ExUnit.configure` line in the test suite. + +```elixir +# ExUnit.configure exclude: :pending, trace: true +``` + +If you're stuck on something, it may help to look at some of +the [available resources](https://exercism.io/tracks/elixir/resources) +out there where answers might be found. + +## Source + +[https://en.wikipedia.org/wiki/Two-fer](https://en.wikipedia.org/wiki/Two-fer) + +## Submitting Incomplete Solutions +It's possible to submit an incomplete solution so you can see how others have completed the exercise. diff --git a/test_data/two_fer/missing_file_solution/test/test.exs b/test_data/two_fer/missing_file_solution/test/test.exs new file mode 100644 index 00000000..cc2cb203 --- /dev/null +++ b/test_data/two_fer/missing_file_solution/test/test.exs @@ -0,0 +1,10 @@ +ExUnit.start(autorun: false) +ExUnit.configure(exclude: :pending, trace: true) + +Code.require_file("two_fer.exs", __DIR__) +Code.require_file("two_fer_test.exs", __DIR__) + +ExUnit.Server.modules_loaded() + +ExUnit.run() + diff --git a/test_data/two_fer/missing_file_solution/test/two_fer_test.exs b/test_data/two_fer/missing_file_solution/test/two_fer_test.exs new file mode 100644 index 00000000..9c9c312c --- /dev/null +++ b/test_data/two_fer/missing_file_solution/test/two_fer_test.exs @@ -0,0 +1,33 @@ +defmodule TwoFerTest do + use ExUnit.Case + + test "no name given" do + assert TwoFer.two_fer() == "One for you, one for me" + end + + @tag :pending + test "a name given" do + assert TwoFer.two_fer("Gilberto Barrosi") == "One for Gilberto Barros, one for me" + end + + @tag :pending + test "when the parameter is a number" do + assert_raise FunctionClauseError, fn -> + TwoFer.two_fer(10) + end + end + + @tag :pending + test "when the parameter is an atom" do + assert_raise FunctionClauseError, fn -> + TwoFer.two_fer(:bob) + end + end + + @tag :pending + test "when the parameter is a charlist" do + assert_raise FunctionClauseError, fn -> + refute TwoFer.two_fer('Jon Snow') + end + end +end From 3243405d7a28f992a5cf1d94e5cae9592f643113 Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Thu, 8 Jul 2021 22:14:42 +0200 Subject: [PATCH 02/16] Remove mentions of statuses, approval etc. --- .../test_suite/take_a_number_test.exs | 6 +- .../test_suite/two_fer_test.exs | 19 ++----- test/elixir_analyzer_test.exs | 8 +-- test/support/exercise_test_case.ex | 4 +- .../.meta/config.json | 0 .../README.md | 0 .../lib/two_fer.ex | 0 .../test/test.exs | 0 .../test/two_fer_test.exs | 0 .../.meta/config.json | 0 .../README.md | 0 .../lib/two_fer.ex | 0 .../test/test.exs | 0 .../test/two_fer_test.exs | 0 .../referred_solution/.meta/config.json | 10 ---- test_data/two_fer/referred_solution/README.md | 55 ------------------- .../two_fer/referred_solution/lib/two_fer.ex | 18 ------ .../two_fer/referred_solution/test/test.exs | 10 ---- .../referred_solution/test/two_fer_test.exs | 33 ----------- 19 files changed, 12 insertions(+), 151 deletions(-) rename test_data/two_fer/{approved_solution => imperfect_solution}/.meta/config.json (100%) rename test_data/two_fer/{approved_solution => imperfect_solution}/README.md (100%) rename test_data/two_fer/{disapproved_solution => imperfect_solution}/lib/two_fer.ex (100%) rename test_data/two_fer/{approved_solution => imperfect_solution}/test/test.exs (100%) rename test_data/two_fer/{disapproved_solution => imperfect_solution}/test/two_fer_test.exs (100%) rename test_data/two_fer/{disapproved_solution => perfect_solution}/.meta/config.json (100%) rename test_data/two_fer/{disapproved_solution => perfect_solution}/README.md (100%) rename test_data/two_fer/{approved_solution => perfect_solution}/lib/two_fer.ex (100%) rename test_data/two_fer/{disapproved_solution => perfect_solution}/test/test.exs (100%) rename test_data/two_fer/{approved_solution => perfect_solution}/test/two_fer_test.exs (100%) delete mode 100644 test_data/two_fer/referred_solution/.meta/config.json delete mode 100644 test_data/two_fer/referred_solution/README.md delete mode 100644 test_data/two_fer/referred_solution/lib/two_fer.ex delete mode 100644 test_data/two_fer/referred_solution/test/test.exs delete mode 100644 test_data/two_fer/referred_solution/test/two_fer_test.exs diff --git a/test/elixir_analyzer/test_suite/take_a_number_test.exs b/test/elixir_analyzer/test_suite/take_a_number_test.exs index 2a6e138c..c265d1af 100644 --- a/test/elixir_analyzer/test_suite/take_a_number_test.exs +++ b/test/elixir_analyzer/test_suite/take_a_number_test.exs @@ -32,8 +32,7 @@ defmodule ElixirAnalyzer.TestSuite.TakeANumberTest do describe "forbids abstractions" do test_exercise_analysis "detects Agent", - comments_include: [Constants.take_a_number_do_not_use_abstractions()], - status: :disapprove do + comments_include: [Constants.take_a_number_do_not_use_abstractions()] do defmodule TakeANumber do use Agent @@ -63,8 +62,7 @@ defmodule ElixirAnalyzer.TestSuite.TakeANumberTest do end test_exercise_analysis "detects GenServer", - comments_include: [Constants.take_a_number_do_not_use_abstractions()], - status: :disapprove do + comments_include: [Constants.take_a_number_do_not_use_abstractions()] do defmodule TakeANumber do use GenServer diff --git a/test/elixir_analyzer/test_suite/two_fer_test.exs b/test/elixir_analyzer/test_suite/two_fer_test.exs index 9fa54c06..73fd199f 100644 --- a/test/elixir_analyzer/test_suite/two_fer_test.exs +++ b/test/elixir_analyzer/test_suite/two_fer_test.exs @@ -16,7 +16,6 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end test_exercise_analysis "missing moduledoc", - status: :approve, comments: [Constants.solution_use_moduledoc()] do defmodule TwoFer do @spec two_fer(String.t()) :: String.t() @@ -38,8 +37,7 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end end - test_exercise_analysis "refer when wrong spec", - status: :refer, + test_exercise_analysis "when wrong spec", comments_include: [Constants.two_fer_wrong_specification()] do [ defmodule TwoFer do @@ -54,7 +52,6 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end test_exercise_analysis "info when missing spec", - status: :approve, comments_include: [Constants.solution_use_specification()] do defmodule TwoFer do @moduledoc """ @@ -82,8 +79,7 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do ] end - test_exercise_analysis "disapprove when not using a default parameter", - status: :disapprove, + test_exercise_analysis "when not using a default parameter", comments_include: [Constants.two_fer_use_default_parameter()] do [ defmodule TwoFer do @@ -114,7 +110,6 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do describe "function header" do test_exercise_analysis "refer when using a function header", - status: :refer, comments_include: [Constants.two_fer_use_of_function_header()] do defmodule TwoFer do def two_fer(name \\ "you") @@ -124,7 +119,6 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do describe "guards" do test_exercise_analysis "usage of is_binary or is_bitstring is required", - status: :disapprove, comments_include: [Constants.two_fer_use_guards()] do defmodule TwoFer do def two_fer(name \\ "you") do @@ -133,8 +127,7 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end end - test_exercise_analysis "refer when is_binary or is_bitstring in used outside of the function head", - status: :refer, + test_exercise_analysis "when is_binary or is_bitstring in used outside of the function head", comments_include: [Constants.two_fer_use_function_level_guard()] do [ defmodule TwoFer do @@ -171,8 +164,7 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end test_exercise_analysis "refer there are other functions than two_fer/1", - comments_include: [Constants.two_fer_use_of_aux_functions()], - status: :refer do + comments_include: [Constants.two_fer_use_of_aux_functions()] do [ defmodule TwoFer do def two_fer(name \\ "you") do @@ -209,8 +201,7 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do describe "string interpolation" do test_exercise_analysis "doesn't allow string concatenation", - comments_include: [Constants.two_fer_use_string_interpolation()], - status: :disapprove do + comments_include: [Constants.two_fer_use_string_interpolation()] do defmodule TwoFer do def two_fer(name \\ "you") when is_binary(name) do "One for " <> name <> ", one for me" diff --git a/test/elixir_analyzer_test.exs b/test/elixir_analyzer_test.exs index d8012c74..6d0a5510 100644 --- a/test/elixir_analyzer_test.exs +++ b/test/elixir_analyzer_test.exs @@ -8,9 +8,9 @@ defmodule ElixirAnalyzerTest do @options [puts_summary: false, write_results: false] # @tag :pending - test "approved solution" do + test "solution with no comments" do exercise = "two-fer" - path = "./test_data/two_fer/approved_solution/" + path = "./test_data/two_fer/perfect_solution/" analyzed_exercise = ElixirAnalyzer.analyze_exercise(exercise, path, path, @options) expected_output = """ @@ -21,9 +21,9 @@ defmodule ElixirAnalyzerTest do end # @tag :pending - test "referred solution with comments" do + test "solution with some comments" do exercise = "two-fer" - path = "./test_data/two_fer/referred_solution/" + path = "./test_data/two_fer/imperfect_solution/" analyzed_exercise = ElixirAnalyzer.analyze_exercise(exercise, path, path, @options) expected_output = """ diff --git a/test/support/exercise_test_case.ex b/test/support/exercise_test_case.ex index cca2cb76..9c275111 100644 --- a/test/support/exercise_test_case.ex +++ b/test/support/exercise_test_case.ex @@ -29,7 +29,6 @@ defmodule ElixirAnalyzer.ExerciseTestCase do ``` test_exercise_analysis "missing moduledoc", - status: :approve, comments: [Constants.solution_use_moduledoc()] do defmodule TwoFer do @spec two_fer(String.t()) :: String.t() @@ -44,7 +43,6 @@ defmodule ElixirAnalyzer.ExerciseTestCase do All assertions are optional, but at least one is required. - - `:status` - an atom, e.g. `:approve`, `:refer`, `:disapprove`. - `:comments` - checks that the comments produced by the analysis and this list have the same elements, ignoring their order. - `:comments_include` - checks that the comments produced by the analysis include all elements from this list. - `:comments_exclude` - checks that the comments produced by the analysis include none of the elements from this list. @@ -55,7 +53,7 @@ defmodule ElixirAnalyzer.ExerciseTestCase do Passing a list of code blocks is also supported. """ defmacro test_exercise_analysis(name, assertions, do: test_cases) do - supported_assertions_keys = [:comments, :comments_include, :comments_exclude, :status] + supported_assertions_keys = [:comments, :comments_include, :comments_exclude] assertions_keys = Keyword.keys(assertions) assertions_key_diff = assertions_keys -- supported_assertions_keys diff --git a/test_data/two_fer/approved_solution/.meta/config.json b/test_data/two_fer/imperfect_solution/.meta/config.json similarity index 100% rename from test_data/two_fer/approved_solution/.meta/config.json rename to test_data/two_fer/imperfect_solution/.meta/config.json diff --git a/test_data/two_fer/approved_solution/README.md b/test_data/two_fer/imperfect_solution/README.md similarity index 100% rename from test_data/two_fer/approved_solution/README.md rename to test_data/two_fer/imperfect_solution/README.md diff --git a/test_data/two_fer/disapproved_solution/lib/two_fer.ex b/test_data/two_fer/imperfect_solution/lib/two_fer.ex similarity index 100% rename from test_data/two_fer/disapproved_solution/lib/two_fer.ex rename to test_data/two_fer/imperfect_solution/lib/two_fer.ex diff --git a/test_data/two_fer/approved_solution/test/test.exs b/test_data/two_fer/imperfect_solution/test/test.exs similarity index 100% rename from test_data/two_fer/approved_solution/test/test.exs rename to test_data/two_fer/imperfect_solution/test/test.exs diff --git a/test_data/two_fer/disapproved_solution/test/two_fer_test.exs b/test_data/two_fer/imperfect_solution/test/two_fer_test.exs similarity index 100% rename from test_data/two_fer/disapproved_solution/test/two_fer_test.exs rename to test_data/two_fer/imperfect_solution/test/two_fer_test.exs diff --git a/test_data/two_fer/disapproved_solution/.meta/config.json b/test_data/two_fer/perfect_solution/.meta/config.json similarity index 100% rename from test_data/two_fer/disapproved_solution/.meta/config.json rename to test_data/two_fer/perfect_solution/.meta/config.json diff --git a/test_data/two_fer/disapproved_solution/README.md b/test_data/two_fer/perfect_solution/README.md similarity index 100% rename from test_data/two_fer/disapproved_solution/README.md rename to test_data/two_fer/perfect_solution/README.md diff --git a/test_data/two_fer/approved_solution/lib/two_fer.ex b/test_data/two_fer/perfect_solution/lib/two_fer.ex similarity index 100% rename from test_data/two_fer/approved_solution/lib/two_fer.ex rename to test_data/two_fer/perfect_solution/lib/two_fer.ex diff --git a/test_data/two_fer/disapproved_solution/test/test.exs b/test_data/two_fer/perfect_solution/test/test.exs similarity index 100% rename from test_data/two_fer/disapproved_solution/test/test.exs rename to test_data/two_fer/perfect_solution/test/test.exs diff --git a/test_data/two_fer/approved_solution/test/two_fer_test.exs b/test_data/two_fer/perfect_solution/test/two_fer_test.exs similarity index 100% rename from test_data/two_fer/approved_solution/test/two_fer_test.exs rename to test_data/two_fer/perfect_solution/test/two_fer_test.exs diff --git a/test_data/two_fer/referred_solution/.meta/config.json b/test_data/two_fer/referred_solution/.meta/config.json deleted file mode 100644 index ed319ba5..00000000 --- a/test_data/two_fer/referred_solution/.meta/config.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "blurb": "Create a sentence of the form \"One for X, one for me.\"", - "authors": [], - "files": { - "solution": ["lib/two_fer.ex"], - "test": ["test/two_fer_test.exs"], - "example": [".meta/example.ex"] - }, - "source_url": "https://github.com/exercism/problem-specifications/issues/757" -} diff --git a/test_data/two_fer/referred_solution/README.md b/test_data/two_fer/referred_solution/README.md deleted file mode 100644 index ce9d6b19..00000000 --- a/test_data/two_fer/referred_solution/README.md +++ /dev/null @@ -1,55 +0,0 @@ -# Two Fer - -`Two-fer` or `2-fer` is short for two for one. One for you and one for me. - -```text -"One for X, one for me." -``` - -When X is a name or "you". - -If the given name is "Alice", the result should be "One for Alice, one for me." -If no name is given, the result should be "One for you, one for me." - - -## Running tests - -Execute the tests with: - -```bash -$ elixir two_fer_test.exs -``` - -### Pending tests - -In the test suites, all but the first test have been skipped. - -Once you get a test passing, you can unskip the next one by -commenting out the relevant `@tag :pending` with a `#` symbol. - -For example: - -```elixir -# @tag :pending -test "shouting" do - assert Bob.hey("WATCH OUT!") == "Whoa, chill out!" -end -``` - -Or, you can enable all the tests by commenting out the -`ExUnit.configure` line in the test suite. - -```elixir -# ExUnit.configure exclude: :pending, trace: true -``` - -If you're stuck on something, it may help to look at some of -the [available resources](https://exercism.io/tracks/elixir/resources) -out there where answers might be found. - -## Source - -[https://en.wikipedia.org/wiki/Two-fer](https://en.wikipedia.org/wiki/Two-fer) - -## Submitting Incomplete Solutions -It's possible to submit an incomplete solution so you can see how others have completed the exercise. diff --git a/test_data/two_fer/referred_solution/lib/two_fer.ex b/test_data/two_fer/referred_solution/lib/two_fer.ex deleted file mode 100644 index bd4dfe6a..00000000 --- a/test_data/two_fer/referred_solution/lib/two_fer.ex +++ /dev/null @@ -1,18 +0,0 @@ -defmodule TwoFer do - @doc """ - Two-fer or 2-fer is short for two for one. One for you and one for me. - """ - def two_fer(name \\ "you") - def two_fer(name) when is_binary(name) do - "One for #{name}, one for me" - end - def two_fer(_name), do: raise FunctionClauseError -end - - - - - - - - diff --git a/test_data/two_fer/referred_solution/test/test.exs b/test_data/two_fer/referred_solution/test/test.exs deleted file mode 100644 index cc2cb203..00000000 --- a/test_data/two_fer/referred_solution/test/test.exs +++ /dev/null @@ -1,10 +0,0 @@ -ExUnit.start(autorun: false) -ExUnit.configure(exclude: :pending, trace: true) - -Code.require_file("two_fer.exs", __DIR__) -Code.require_file("two_fer_test.exs", __DIR__) - -ExUnit.Server.modules_loaded() - -ExUnit.run() - diff --git a/test_data/two_fer/referred_solution/test/two_fer_test.exs b/test_data/two_fer/referred_solution/test/two_fer_test.exs deleted file mode 100644 index 9c9c312c..00000000 --- a/test_data/two_fer/referred_solution/test/two_fer_test.exs +++ /dev/null @@ -1,33 +0,0 @@ -defmodule TwoFerTest do - use ExUnit.Case - - test "no name given" do - assert TwoFer.two_fer() == "One for you, one for me" - end - - @tag :pending - test "a name given" do - assert TwoFer.two_fer("Gilberto Barrosi") == "One for Gilberto Barros, one for me" - end - - @tag :pending - test "when the parameter is a number" do - assert_raise FunctionClauseError, fn -> - TwoFer.two_fer(10) - end - end - - @tag :pending - test "when the parameter is an atom" do - assert_raise FunctionClauseError, fn -> - TwoFer.two_fer(:bob) - end - end - - @tag :pending - test "when the parameter is a charlist" do - assert_raise FunctionClauseError, fn -> - refute TwoFer.two_fer('Jon Snow') - end - end -end From 49648e99532749466b139fadb09f558d4f08349b Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Fri, 9 Jul 2021 11:23:06 +0200 Subject: [PATCH 03/16] Hide expected error message in test --- test/elixir_analyzer_test.exs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test/elixir_analyzer_test.exs b/test/elixir_analyzer_test.exs index 6d0a5510..4c7ccfba 100644 --- a/test/elixir_analyzer_test.exs +++ b/test/elixir_analyzer_test.exs @@ -2,6 +2,8 @@ defmodule ElixirAnalyzerTest do use ExUnit.Case doctest ElixirAnalyzer + import ExUnit.CaptureLog + alias ElixirAnalyzer.Submission describe "ElixirAnalyzer" do @@ -37,13 +39,16 @@ defmodule ElixirAnalyzerTest do test "error solution" do exercise = "two-fer" path = "./test_data/two_fer/error_solution/" - analyzed_exercise = ElixirAnalyzer.analyze_exercise(exercise, path, path, @options) - expected_output = """ - {\"comments\":[{\"comment\":\"elixir.general.parsing_error\",\"params\":{\"error\":\"missing terminator: end (for \\\"do\\\" starting at line 1)\",\"line\":14},\"type\":\"essential\"}],\"summary\":\"Check the comments for things to fix. 🛠\"} - """ + capture_log(fn -> + analyzed_exercise = ElixirAnalyzer.analyze_exercise(exercise, path, path, @options) - assert Submission.to_json(analyzed_exercise) == String.trim(expected_output) + expected_output = """ + {\"comments\":[{\"comment\":\"elixir.general.parsing_error\",\"params\":{\"error\":\"missing terminator: end (for \\\"do\\\" starting at line 1)\",\"line\":14},\"type\":\"essential\"}],\"summary\":\"Check the comments for things to fix. 🛠\"} + """ + + assert Submission.to_json(analyzed_exercise) == String.trim(expected_output) + end) end test "missing file solution" do From 1d252003f277e50413f6550fded003dfa1d6e1c4 Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Fri, 9 Jul 2021 11:26:29 +0200 Subject: [PATCH 04/16] Revert "Remove mentions of statuses, approval etc." This reverts commit 3243405d7a28f992a5cf1d94e5cae9592f643113. --- .../test_suite/take_a_number_test.exs | 6 +- .../test_suite/two_fer_test.exs | 19 +++++-- test/elixir_analyzer_test.exs | 8 +-- test/support/exercise_test_case.ex | 4 +- .../.meta/config.json | 0 .../README.md | 0 .../lib/two_fer.ex | 0 .../test/test.exs | 0 .../test/two_fer_test.exs | 0 .../.meta/config.json | 0 .../README.md | 0 .../lib/two_fer.ex | 0 .../test/test.exs | 0 .../test/two_fer_test.exs | 0 .../referred_solution/.meta/config.json | 10 ++++ test_data/two_fer/referred_solution/README.md | 55 +++++++++++++++++++ .../two_fer/referred_solution/lib/two_fer.ex | 18 ++++++ .../two_fer/referred_solution/test/test.exs | 10 ++++ .../referred_solution/test/two_fer_test.exs | 33 +++++++++++ 19 files changed, 151 insertions(+), 12 deletions(-) rename test_data/two_fer/{imperfect_solution => approved_solution}/.meta/config.json (100%) rename test_data/two_fer/{imperfect_solution => approved_solution}/README.md (100%) rename test_data/two_fer/{perfect_solution => approved_solution}/lib/two_fer.ex (100%) rename test_data/two_fer/{imperfect_solution => approved_solution}/test/test.exs (100%) rename test_data/two_fer/{perfect_solution => approved_solution}/test/two_fer_test.exs (100%) rename test_data/two_fer/{perfect_solution => disapproved_solution}/.meta/config.json (100%) rename test_data/two_fer/{perfect_solution => disapproved_solution}/README.md (100%) rename test_data/two_fer/{imperfect_solution => disapproved_solution}/lib/two_fer.ex (100%) rename test_data/two_fer/{perfect_solution => disapproved_solution}/test/test.exs (100%) rename test_data/two_fer/{imperfect_solution => disapproved_solution}/test/two_fer_test.exs (100%) create mode 100644 test_data/two_fer/referred_solution/.meta/config.json create mode 100644 test_data/two_fer/referred_solution/README.md create mode 100644 test_data/two_fer/referred_solution/lib/two_fer.ex create mode 100644 test_data/two_fer/referred_solution/test/test.exs create mode 100644 test_data/two_fer/referred_solution/test/two_fer_test.exs diff --git a/test/elixir_analyzer/test_suite/take_a_number_test.exs b/test/elixir_analyzer/test_suite/take_a_number_test.exs index c265d1af..2a6e138c 100644 --- a/test/elixir_analyzer/test_suite/take_a_number_test.exs +++ b/test/elixir_analyzer/test_suite/take_a_number_test.exs @@ -32,7 +32,8 @@ defmodule ElixirAnalyzer.TestSuite.TakeANumberTest do describe "forbids abstractions" do test_exercise_analysis "detects Agent", - comments_include: [Constants.take_a_number_do_not_use_abstractions()] do + comments_include: [Constants.take_a_number_do_not_use_abstractions()], + status: :disapprove do defmodule TakeANumber do use Agent @@ -62,7 +63,8 @@ defmodule ElixirAnalyzer.TestSuite.TakeANumberTest do end test_exercise_analysis "detects GenServer", - comments_include: [Constants.take_a_number_do_not_use_abstractions()] do + comments_include: [Constants.take_a_number_do_not_use_abstractions()], + status: :disapprove do defmodule TakeANumber do use GenServer diff --git a/test/elixir_analyzer/test_suite/two_fer_test.exs b/test/elixir_analyzer/test_suite/two_fer_test.exs index 73fd199f..9fa54c06 100644 --- a/test/elixir_analyzer/test_suite/two_fer_test.exs +++ b/test/elixir_analyzer/test_suite/two_fer_test.exs @@ -16,6 +16,7 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end test_exercise_analysis "missing moduledoc", + status: :approve, comments: [Constants.solution_use_moduledoc()] do defmodule TwoFer do @spec two_fer(String.t()) :: String.t() @@ -37,7 +38,8 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end end - test_exercise_analysis "when wrong spec", + test_exercise_analysis "refer when wrong spec", + status: :refer, comments_include: [Constants.two_fer_wrong_specification()] do [ defmodule TwoFer do @@ -52,6 +54,7 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end test_exercise_analysis "info when missing spec", + status: :approve, comments_include: [Constants.solution_use_specification()] do defmodule TwoFer do @moduledoc """ @@ -79,7 +82,8 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do ] end - test_exercise_analysis "when not using a default parameter", + test_exercise_analysis "disapprove when not using a default parameter", + status: :disapprove, comments_include: [Constants.two_fer_use_default_parameter()] do [ defmodule TwoFer do @@ -110,6 +114,7 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do describe "function header" do test_exercise_analysis "refer when using a function header", + status: :refer, comments_include: [Constants.two_fer_use_of_function_header()] do defmodule TwoFer do def two_fer(name \\ "you") @@ -119,6 +124,7 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do describe "guards" do test_exercise_analysis "usage of is_binary or is_bitstring is required", + status: :disapprove, comments_include: [Constants.two_fer_use_guards()] do defmodule TwoFer do def two_fer(name \\ "you") do @@ -127,7 +133,8 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end end - test_exercise_analysis "when is_binary or is_bitstring in used outside of the function head", + test_exercise_analysis "refer when is_binary or is_bitstring in used outside of the function head", + status: :refer, comments_include: [Constants.two_fer_use_function_level_guard()] do [ defmodule TwoFer do @@ -164,7 +171,8 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end test_exercise_analysis "refer there are other functions than two_fer/1", - comments_include: [Constants.two_fer_use_of_aux_functions()] do + comments_include: [Constants.two_fer_use_of_aux_functions()], + status: :refer do [ defmodule TwoFer do def two_fer(name \\ "you") do @@ -201,7 +209,8 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do describe "string interpolation" do test_exercise_analysis "doesn't allow string concatenation", - comments_include: [Constants.two_fer_use_string_interpolation()] do + comments_include: [Constants.two_fer_use_string_interpolation()], + status: :disapprove do defmodule TwoFer do def two_fer(name \\ "you") when is_binary(name) do "One for " <> name <> ", one for me" diff --git a/test/elixir_analyzer_test.exs b/test/elixir_analyzer_test.exs index 4c7ccfba..2f338535 100644 --- a/test/elixir_analyzer_test.exs +++ b/test/elixir_analyzer_test.exs @@ -10,9 +10,9 @@ defmodule ElixirAnalyzerTest do @options [puts_summary: false, write_results: false] # @tag :pending - test "solution with no comments" do + test "approved solution" do exercise = "two-fer" - path = "./test_data/two_fer/perfect_solution/" + path = "./test_data/two_fer/approved_solution/" analyzed_exercise = ElixirAnalyzer.analyze_exercise(exercise, path, path, @options) expected_output = """ @@ -23,9 +23,9 @@ defmodule ElixirAnalyzerTest do end # @tag :pending - test "solution with some comments" do + test "referred solution with comments" do exercise = "two-fer" - path = "./test_data/two_fer/imperfect_solution/" + path = "./test_data/two_fer/referred_solution/" analyzed_exercise = ElixirAnalyzer.analyze_exercise(exercise, path, path, @options) expected_output = """ diff --git a/test/support/exercise_test_case.ex b/test/support/exercise_test_case.ex index 9c275111..cca2cb76 100644 --- a/test/support/exercise_test_case.ex +++ b/test/support/exercise_test_case.ex @@ -29,6 +29,7 @@ defmodule ElixirAnalyzer.ExerciseTestCase do ``` test_exercise_analysis "missing moduledoc", + status: :approve, comments: [Constants.solution_use_moduledoc()] do defmodule TwoFer do @spec two_fer(String.t()) :: String.t() @@ -43,6 +44,7 @@ defmodule ElixirAnalyzer.ExerciseTestCase do All assertions are optional, but at least one is required. + - `:status` - an atom, e.g. `:approve`, `:refer`, `:disapprove`. - `:comments` - checks that the comments produced by the analysis and this list have the same elements, ignoring their order. - `:comments_include` - checks that the comments produced by the analysis include all elements from this list. - `:comments_exclude` - checks that the comments produced by the analysis include none of the elements from this list. @@ -53,7 +55,7 @@ defmodule ElixirAnalyzer.ExerciseTestCase do Passing a list of code blocks is also supported. """ defmacro test_exercise_analysis(name, assertions, do: test_cases) do - supported_assertions_keys = [:comments, :comments_include, :comments_exclude] + supported_assertions_keys = [:comments, :comments_include, :comments_exclude, :status] assertions_keys = Keyword.keys(assertions) assertions_key_diff = assertions_keys -- supported_assertions_keys diff --git a/test_data/two_fer/imperfect_solution/.meta/config.json b/test_data/two_fer/approved_solution/.meta/config.json similarity index 100% rename from test_data/two_fer/imperfect_solution/.meta/config.json rename to test_data/two_fer/approved_solution/.meta/config.json diff --git a/test_data/two_fer/imperfect_solution/README.md b/test_data/two_fer/approved_solution/README.md similarity index 100% rename from test_data/two_fer/imperfect_solution/README.md rename to test_data/two_fer/approved_solution/README.md diff --git a/test_data/two_fer/perfect_solution/lib/two_fer.ex b/test_data/two_fer/approved_solution/lib/two_fer.ex similarity index 100% rename from test_data/two_fer/perfect_solution/lib/two_fer.ex rename to test_data/two_fer/approved_solution/lib/two_fer.ex diff --git a/test_data/two_fer/imperfect_solution/test/test.exs b/test_data/two_fer/approved_solution/test/test.exs similarity index 100% rename from test_data/two_fer/imperfect_solution/test/test.exs rename to test_data/two_fer/approved_solution/test/test.exs diff --git a/test_data/two_fer/perfect_solution/test/two_fer_test.exs b/test_data/two_fer/approved_solution/test/two_fer_test.exs similarity index 100% rename from test_data/two_fer/perfect_solution/test/two_fer_test.exs rename to test_data/two_fer/approved_solution/test/two_fer_test.exs diff --git a/test_data/two_fer/perfect_solution/.meta/config.json b/test_data/two_fer/disapproved_solution/.meta/config.json similarity index 100% rename from test_data/two_fer/perfect_solution/.meta/config.json rename to test_data/two_fer/disapproved_solution/.meta/config.json diff --git a/test_data/two_fer/perfect_solution/README.md b/test_data/two_fer/disapproved_solution/README.md similarity index 100% rename from test_data/two_fer/perfect_solution/README.md rename to test_data/two_fer/disapproved_solution/README.md diff --git a/test_data/two_fer/imperfect_solution/lib/two_fer.ex b/test_data/two_fer/disapproved_solution/lib/two_fer.ex similarity index 100% rename from test_data/two_fer/imperfect_solution/lib/two_fer.ex rename to test_data/two_fer/disapproved_solution/lib/two_fer.ex diff --git a/test_data/two_fer/perfect_solution/test/test.exs b/test_data/two_fer/disapproved_solution/test/test.exs similarity index 100% rename from test_data/two_fer/perfect_solution/test/test.exs rename to test_data/two_fer/disapproved_solution/test/test.exs diff --git a/test_data/two_fer/imperfect_solution/test/two_fer_test.exs b/test_data/two_fer/disapproved_solution/test/two_fer_test.exs similarity index 100% rename from test_data/two_fer/imperfect_solution/test/two_fer_test.exs rename to test_data/two_fer/disapproved_solution/test/two_fer_test.exs diff --git a/test_data/two_fer/referred_solution/.meta/config.json b/test_data/two_fer/referred_solution/.meta/config.json new file mode 100644 index 00000000..ed319ba5 --- /dev/null +++ b/test_data/two_fer/referred_solution/.meta/config.json @@ -0,0 +1,10 @@ +{ + "blurb": "Create a sentence of the form \"One for X, one for me.\"", + "authors": [], + "files": { + "solution": ["lib/two_fer.ex"], + "test": ["test/two_fer_test.exs"], + "example": [".meta/example.ex"] + }, + "source_url": "https://github.com/exercism/problem-specifications/issues/757" +} diff --git a/test_data/two_fer/referred_solution/README.md b/test_data/two_fer/referred_solution/README.md new file mode 100644 index 00000000..ce9d6b19 --- /dev/null +++ b/test_data/two_fer/referred_solution/README.md @@ -0,0 +1,55 @@ +# Two Fer + +`Two-fer` or `2-fer` is short for two for one. One for you and one for me. + +```text +"One for X, one for me." +``` + +When X is a name or "you". + +If the given name is "Alice", the result should be "One for Alice, one for me." +If no name is given, the result should be "One for you, one for me." + + +## Running tests + +Execute the tests with: + +```bash +$ elixir two_fer_test.exs +``` + +### Pending tests + +In the test suites, all but the first test have been skipped. + +Once you get a test passing, you can unskip the next one by +commenting out the relevant `@tag :pending` with a `#` symbol. + +For example: + +```elixir +# @tag :pending +test "shouting" do + assert Bob.hey("WATCH OUT!") == "Whoa, chill out!" +end +``` + +Or, you can enable all the tests by commenting out the +`ExUnit.configure` line in the test suite. + +```elixir +# ExUnit.configure exclude: :pending, trace: true +``` + +If you're stuck on something, it may help to look at some of +the [available resources](https://exercism.io/tracks/elixir/resources) +out there where answers might be found. + +## Source + +[https://en.wikipedia.org/wiki/Two-fer](https://en.wikipedia.org/wiki/Two-fer) + +## Submitting Incomplete Solutions +It's possible to submit an incomplete solution so you can see how others have completed the exercise. diff --git a/test_data/two_fer/referred_solution/lib/two_fer.ex b/test_data/two_fer/referred_solution/lib/two_fer.ex new file mode 100644 index 00000000..bd4dfe6a --- /dev/null +++ b/test_data/two_fer/referred_solution/lib/two_fer.ex @@ -0,0 +1,18 @@ +defmodule TwoFer do + @doc """ + Two-fer or 2-fer is short for two for one. One for you and one for me. + """ + def two_fer(name \\ "you") + def two_fer(name) when is_binary(name) do + "One for #{name}, one for me" + end + def two_fer(_name), do: raise FunctionClauseError +end + + + + + + + + diff --git a/test_data/two_fer/referred_solution/test/test.exs b/test_data/two_fer/referred_solution/test/test.exs new file mode 100644 index 00000000..cc2cb203 --- /dev/null +++ b/test_data/two_fer/referred_solution/test/test.exs @@ -0,0 +1,10 @@ +ExUnit.start(autorun: false) +ExUnit.configure(exclude: :pending, trace: true) + +Code.require_file("two_fer.exs", __DIR__) +Code.require_file("two_fer_test.exs", __DIR__) + +ExUnit.Server.modules_loaded() + +ExUnit.run() + diff --git a/test_data/two_fer/referred_solution/test/two_fer_test.exs b/test_data/two_fer/referred_solution/test/two_fer_test.exs new file mode 100644 index 00000000..9c9c312c --- /dev/null +++ b/test_data/two_fer/referred_solution/test/two_fer_test.exs @@ -0,0 +1,33 @@ +defmodule TwoFerTest do + use ExUnit.Case + + test "no name given" do + assert TwoFer.two_fer() == "One for you, one for me" + end + + @tag :pending + test "a name given" do + assert TwoFer.two_fer("Gilberto Barrosi") == "One for Gilberto Barros, one for me" + end + + @tag :pending + test "when the parameter is a number" do + assert_raise FunctionClauseError, fn -> + TwoFer.two_fer(10) + end + end + + @tag :pending + test "when the parameter is an atom" do + assert_raise FunctionClauseError, fn -> + TwoFer.two_fer(:bob) + end + end + + @tag :pending + test "when the parameter is a charlist" do + assert_raise FunctionClauseError, fn -> + refute TwoFer.two_fer('Jon Snow') + end + end +end From b9d84bb0c1bdd7c07c61fffed3531eadb436e9b6 Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Fri, 9 Jul 2021 11:30:16 +0200 Subject: [PATCH 05/16] Remove mentions of statuses, approval etc. part 1 --- .../test_suite/take_a_number_test.exs | 6 ++---- test/elixir_analyzer/test_suite/two_fer_test.exs | 13 ++----------- test/support/exercise_test_case.ex | 4 +--- 3 files changed, 5 insertions(+), 18 deletions(-) diff --git a/test/elixir_analyzer/test_suite/take_a_number_test.exs b/test/elixir_analyzer/test_suite/take_a_number_test.exs index 2a6e138c..c265d1af 100644 --- a/test/elixir_analyzer/test_suite/take_a_number_test.exs +++ b/test/elixir_analyzer/test_suite/take_a_number_test.exs @@ -32,8 +32,7 @@ defmodule ElixirAnalyzer.TestSuite.TakeANumberTest do describe "forbids abstractions" do test_exercise_analysis "detects Agent", - comments_include: [Constants.take_a_number_do_not_use_abstractions()], - status: :disapprove do + comments_include: [Constants.take_a_number_do_not_use_abstractions()] do defmodule TakeANumber do use Agent @@ -63,8 +62,7 @@ defmodule ElixirAnalyzer.TestSuite.TakeANumberTest do end test_exercise_analysis "detects GenServer", - comments_include: [Constants.take_a_number_do_not_use_abstractions()], - status: :disapprove do + comments_include: [Constants.take_a_number_do_not_use_abstractions()] do defmodule TakeANumber do use GenServer diff --git a/test/elixir_analyzer/test_suite/two_fer_test.exs b/test/elixir_analyzer/test_suite/two_fer_test.exs index 9fa54c06..20434027 100644 --- a/test/elixir_analyzer/test_suite/two_fer_test.exs +++ b/test/elixir_analyzer/test_suite/two_fer_test.exs @@ -16,7 +16,6 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end test_exercise_analysis "missing moduledoc", - status: :approve, comments: [Constants.solution_use_moduledoc()] do defmodule TwoFer do @spec two_fer(String.t()) :: String.t() @@ -39,7 +38,6 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end test_exercise_analysis "refer when wrong spec", - status: :refer, comments_include: [Constants.two_fer_wrong_specification()] do [ defmodule TwoFer do @@ -54,7 +52,6 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end test_exercise_analysis "info when missing spec", - status: :approve, comments_include: [Constants.solution_use_specification()] do defmodule TwoFer do @moduledoc """ @@ -83,7 +80,6 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end test_exercise_analysis "disapprove when not using a default parameter", - status: :disapprove, comments_include: [Constants.two_fer_use_default_parameter()] do [ defmodule TwoFer do @@ -114,7 +110,6 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do describe "function header" do test_exercise_analysis "refer when using a function header", - status: :refer, comments_include: [Constants.two_fer_use_of_function_header()] do defmodule TwoFer do def two_fer(name \\ "you") @@ -124,7 +119,6 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do describe "guards" do test_exercise_analysis "usage of is_binary or is_bitstring is required", - status: :disapprove, comments_include: [Constants.two_fer_use_guards()] do defmodule TwoFer do def two_fer(name \\ "you") do @@ -134,7 +128,6 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end test_exercise_analysis "refer when is_binary or is_bitstring in used outside of the function head", - status: :refer, comments_include: [Constants.two_fer_use_function_level_guard()] do [ defmodule TwoFer do @@ -171,8 +164,7 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do end test_exercise_analysis "refer there are other functions than two_fer/1", - comments_include: [Constants.two_fer_use_of_aux_functions()], - status: :refer do + comments_include: [Constants.two_fer_use_of_aux_functions()] do [ defmodule TwoFer do def two_fer(name \\ "you") do @@ -209,8 +201,7 @@ defmodule ElixirAnalyzer.TestSuite.TwoFerTest do describe "string interpolation" do test_exercise_analysis "doesn't allow string concatenation", - comments_include: [Constants.two_fer_use_string_interpolation()], - status: :disapprove do + comments_include: [Constants.two_fer_use_string_interpolation()] do defmodule TwoFer do def two_fer(name \\ "you") when is_binary(name) do "One for " <> name <> ", one for me" diff --git a/test/support/exercise_test_case.ex b/test/support/exercise_test_case.ex index cca2cb76..9c275111 100644 --- a/test/support/exercise_test_case.ex +++ b/test/support/exercise_test_case.ex @@ -29,7 +29,6 @@ defmodule ElixirAnalyzer.ExerciseTestCase do ``` test_exercise_analysis "missing moduledoc", - status: :approve, comments: [Constants.solution_use_moduledoc()] do defmodule TwoFer do @spec two_fer(String.t()) :: String.t() @@ -44,7 +43,6 @@ defmodule ElixirAnalyzer.ExerciseTestCase do All assertions are optional, but at least one is required. - - `:status` - an atom, e.g. `:approve`, `:refer`, `:disapprove`. - `:comments` - checks that the comments produced by the analysis and this list have the same elements, ignoring their order. - `:comments_include` - checks that the comments produced by the analysis include all elements from this list. - `:comments_exclude` - checks that the comments produced by the analysis include none of the elements from this list. @@ -55,7 +53,7 @@ defmodule ElixirAnalyzer.ExerciseTestCase do Passing a list of code blocks is also supported. """ defmacro test_exercise_analysis(name, assertions, do: test_cases) do - supported_assertions_keys = [:comments, :comments_include, :comments_exclude, :status] + supported_assertions_keys = [:comments, :comments_include, :comments_exclude] assertions_keys = Keyword.keys(assertions) assertions_key_diff = assertions_keys -- supported_assertions_keys From b67f29db41b7c416ebc9662af752deabcdd98ae5 Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Fri, 9 Jul 2021 11:32:08 +0200 Subject: [PATCH 06/16] Hide expected error message in test again --- test/elixir_analyzer_test.exs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/test/elixir_analyzer_test.exs b/test/elixir_analyzer_test.exs index 2f338535..dcd18e43 100644 --- a/test/elixir_analyzer_test.exs +++ b/test/elixir_analyzer_test.exs @@ -66,13 +66,16 @@ defmodule ElixirAnalyzerTest do test "solution for an exercise with no analyzer module" do exercise = "not-a-real-exercise" path = "./test_data/two_fer/error_solution/" - analyzed_exercise = ElixirAnalyzer.analyze_exercise(exercise, path, path, @options) - expected_output = """ - {\"comments\":[],\"summary\":\"Analysis was halted. Analysis skipped, no analysis suite exists for this exercise\"} - """ + capture_log(fn -> + analyzed_exercise = ElixirAnalyzer.analyze_exercise(exercise, path, path, @options) - assert Submission.to_json(analyzed_exercise) == String.trim(expected_output) + expected_output = """ + {\"comments\":[],\"summary\":\"Analysis was halted. Analysis skipped, no analysis suite exists for this exercise\"} + """ + + assert Submission.to_json(analyzed_exercise) == String.trim(expected_output) + end) end end From 0a9c5b9e53c96a4adc5d27188a26d47ade9365eb Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Fri, 9 Jul 2021 11:33:00 +0200 Subject: [PATCH 07/16] Rename approved_solution --- test/elixir_analyzer_test.exs | 4 ++-- .../{approved_solution => perfect_solution}/.meta/config.json | 0 .../two_fer/{approved_solution => perfect_solution}/README.md | 0 .../{approved_solution => perfect_solution}/lib/two_fer.ex | 0 .../{approved_solution => perfect_solution}/test/test.exs | 0 .../test/two_fer_test.exs | 0 6 files changed, 2 insertions(+), 2 deletions(-) rename test_data/two_fer/{approved_solution => perfect_solution}/.meta/config.json (100%) rename test_data/two_fer/{approved_solution => perfect_solution}/README.md (100%) rename test_data/two_fer/{approved_solution => perfect_solution}/lib/two_fer.ex (100%) rename test_data/two_fer/{approved_solution => perfect_solution}/test/test.exs (100%) rename test_data/two_fer/{approved_solution => perfect_solution}/test/two_fer_test.exs (100%) diff --git a/test/elixir_analyzer_test.exs b/test/elixir_analyzer_test.exs index dcd18e43..1cfc32c3 100644 --- a/test/elixir_analyzer_test.exs +++ b/test/elixir_analyzer_test.exs @@ -10,9 +10,9 @@ defmodule ElixirAnalyzerTest do @options [puts_summary: false, write_results: false] # @tag :pending - test "approved solution" do + test "solution with no comments" do exercise = "two-fer" - path = "./test_data/two_fer/approved_solution/" + path = "./test_data/two_fer/perfect_solution/" analyzed_exercise = ElixirAnalyzer.analyze_exercise(exercise, path, path, @options) expected_output = """ diff --git a/test_data/two_fer/approved_solution/.meta/config.json b/test_data/two_fer/perfect_solution/.meta/config.json similarity index 100% rename from test_data/two_fer/approved_solution/.meta/config.json rename to test_data/two_fer/perfect_solution/.meta/config.json diff --git a/test_data/two_fer/approved_solution/README.md b/test_data/two_fer/perfect_solution/README.md similarity index 100% rename from test_data/two_fer/approved_solution/README.md rename to test_data/two_fer/perfect_solution/README.md diff --git a/test_data/two_fer/approved_solution/lib/two_fer.ex b/test_data/two_fer/perfect_solution/lib/two_fer.ex similarity index 100% rename from test_data/two_fer/approved_solution/lib/two_fer.ex rename to test_data/two_fer/perfect_solution/lib/two_fer.ex diff --git a/test_data/two_fer/approved_solution/test/test.exs b/test_data/two_fer/perfect_solution/test/test.exs similarity index 100% rename from test_data/two_fer/approved_solution/test/test.exs rename to test_data/two_fer/perfect_solution/test/test.exs diff --git a/test_data/two_fer/approved_solution/test/two_fer_test.exs b/test_data/two_fer/perfect_solution/test/two_fer_test.exs similarity index 100% rename from test_data/two_fer/approved_solution/test/two_fer_test.exs rename to test_data/two_fer/perfect_solution/test/two_fer_test.exs From 81ef14d40b76896efcfcaea44e4a7a3e986030da Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Fri, 9 Jul 2021 11:33:48 +0200 Subject: [PATCH 08/16] Rename referred_solution --- test/elixir_analyzer_test.exs | 2 +- .../{referred_solution => imperfect_solution}/.meta/config.json | 0 .../two_fer/{referred_solution => imperfect_solution}/README.md | 0 .../{referred_solution => imperfect_solution}/lib/two_fer.ex | 0 .../{referred_solution => imperfect_solution}/test/test.exs | 0 .../test/two_fer_test.exs | 0 6 files changed, 1 insertion(+), 1 deletion(-) rename test_data/two_fer/{referred_solution => imperfect_solution}/.meta/config.json (100%) rename test_data/two_fer/{referred_solution => imperfect_solution}/README.md (100%) rename test_data/two_fer/{referred_solution => imperfect_solution}/lib/two_fer.ex (100%) rename test_data/two_fer/{referred_solution => imperfect_solution}/test/test.exs (100%) rename test_data/two_fer/{referred_solution => imperfect_solution}/test/two_fer_test.exs (100%) diff --git a/test/elixir_analyzer_test.exs b/test/elixir_analyzer_test.exs index 1cfc32c3..26e5fe4f 100644 --- a/test/elixir_analyzer_test.exs +++ b/test/elixir_analyzer_test.exs @@ -25,7 +25,7 @@ defmodule ElixirAnalyzerTest do # @tag :pending test "referred solution with comments" do exercise = "two-fer" - path = "./test_data/two_fer/referred_solution/" + path = "./test_data/two_fer/imperfect_solution/" analyzed_exercise = ElixirAnalyzer.analyze_exercise(exercise, path, path, @options) expected_output = """ diff --git a/test_data/two_fer/referred_solution/.meta/config.json b/test_data/two_fer/imperfect_solution/.meta/config.json similarity index 100% rename from test_data/two_fer/referred_solution/.meta/config.json rename to test_data/two_fer/imperfect_solution/.meta/config.json diff --git a/test_data/two_fer/referred_solution/README.md b/test_data/two_fer/imperfect_solution/README.md similarity index 100% rename from test_data/two_fer/referred_solution/README.md rename to test_data/two_fer/imperfect_solution/README.md diff --git a/test_data/two_fer/referred_solution/lib/two_fer.ex b/test_data/two_fer/imperfect_solution/lib/two_fer.ex similarity index 100% rename from test_data/two_fer/referred_solution/lib/two_fer.ex rename to test_data/two_fer/imperfect_solution/lib/two_fer.ex diff --git a/test_data/two_fer/referred_solution/test/test.exs b/test_data/two_fer/imperfect_solution/test/test.exs similarity index 100% rename from test_data/two_fer/referred_solution/test/test.exs rename to test_data/two_fer/imperfect_solution/test/test.exs diff --git a/test_data/two_fer/referred_solution/test/two_fer_test.exs b/test_data/two_fer/imperfect_solution/test/two_fer_test.exs similarity index 100% rename from test_data/two_fer/referred_solution/test/two_fer_test.exs rename to test_data/two_fer/imperfect_solution/test/two_fer_test.exs From b014ea70dedecdfb6570348e68e0bd9531304005 Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Fri, 9 Jul 2021 11:34:40 +0200 Subject: [PATCH 09/16] Remove unused disapproved_solution --- .../disapproved_solution/.meta/config.json | 10 ---- .../two_fer/disapproved_solution/README.md | 55 ------------------- .../disapproved_solution/lib/two_fer.ex | 17 ------ .../disapproved_solution/test/test.exs | 10 ---- .../test/two_fer_test.exs | 33 ----------- 5 files changed, 125 deletions(-) delete mode 100644 test_data/two_fer/disapproved_solution/.meta/config.json delete mode 100644 test_data/two_fer/disapproved_solution/README.md delete mode 100644 test_data/two_fer/disapproved_solution/lib/two_fer.ex delete mode 100644 test_data/two_fer/disapproved_solution/test/test.exs delete mode 100644 test_data/two_fer/disapproved_solution/test/two_fer_test.exs diff --git a/test_data/two_fer/disapproved_solution/.meta/config.json b/test_data/two_fer/disapproved_solution/.meta/config.json deleted file mode 100644 index ed319ba5..00000000 --- a/test_data/two_fer/disapproved_solution/.meta/config.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "blurb": "Create a sentence of the form \"One for X, one for me.\"", - "authors": [], - "files": { - "solution": ["lib/two_fer.ex"], - "test": ["test/two_fer_test.exs"], - "example": [".meta/example.ex"] - }, - "source_url": "https://github.com/exercism/problem-specifications/issues/757" -} diff --git a/test_data/two_fer/disapproved_solution/README.md b/test_data/two_fer/disapproved_solution/README.md deleted file mode 100644 index ce9d6b19..00000000 --- a/test_data/two_fer/disapproved_solution/README.md +++ /dev/null @@ -1,55 +0,0 @@ -# Two Fer - -`Two-fer` or `2-fer` is short for two for one. One for you and one for me. - -```text -"One for X, one for me." -``` - -When X is a name or "you". - -If the given name is "Alice", the result should be "One for Alice, one for me." -If no name is given, the result should be "One for you, one for me." - - -## Running tests - -Execute the tests with: - -```bash -$ elixir two_fer_test.exs -``` - -### Pending tests - -In the test suites, all but the first test have been skipped. - -Once you get a test passing, you can unskip the next one by -commenting out the relevant `@tag :pending` with a `#` symbol. - -For example: - -```elixir -# @tag :pending -test "shouting" do - assert Bob.hey("WATCH OUT!") == "Whoa, chill out!" -end -``` - -Or, you can enable all the tests by commenting out the -`ExUnit.configure` line in the test suite. - -```elixir -# ExUnit.configure exclude: :pending, trace: true -``` - -If you're stuck on something, it may help to look at some of -the [available resources](https://exercism.io/tracks/elixir/resources) -out there where answers might be found. - -## Source - -[https://en.wikipedia.org/wiki/Two-fer](https://en.wikipedia.org/wiki/Two-fer) - -## Submitting Incomplete Solutions -It's possible to submit an incomplete solution so you can see how others have completed the exercise. diff --git a/test_data/two_fer/disapproved_solution/lib/two_fer.ex b/test_data/two_fer/disapproved_solution/lib/two_fer.ex deleted file mode 100644 index 3977ae70..00000000 --- a/test_data/two_fer/disapproved_solution/lib/two_fer.ex +++ /dev/null @@ -1,17 +0,0 @@ -defmodule TwoFer do - @doc """ - Two-fer or 2-fer is short for two for one. One for you and one for me. - """ - def two_fer(name) when is_binary(name) do - "One for #{name}, one for me" - end - def two_fer(_name), do: raise FunctionClauseError -end - - - - - - - - diff --git a/test_data/two_fer/disapproved_solution/test/test.exs b/test_data/two_fer/disapproved_solution/test/test.exs deleted file mode 100644 index cc2cb203..00000000 --- a/test_data/two_fer/disapproved_solution/test/test.exs +++ /dev/null @@ -1,10 +0,0 @@ -ExUnit.start(autorun: false) -ExUnit.configure(exclude: :pending, trace: true) - -Code.require_file("two_fer.exs", __DIR__) -Code.require_file("two_fer_test.exs", __DIR__) - -ExUnit.Server.modules_loaded() - -ExUnit.run() - diff --git a/test_data/two_fer/disapproved_solution/test/two_fer_test.exs b/test_data/two_fer/disapproved_solution/test/two_fer_test.exs deleted file mode 100644 index 9c9c312c..00000000 --- a/test_data/two_fer/disapproved_solution/test/two_fer_test.exs +++ /dev/null @@ -1,33 +0,0 @@ -defmodule TwoFerTest do - use ExUnit.Case - - test "no name given" do - assert TwoFer.two_fer() == "One for you, one for me" - end - - @tag :pending - test "a name given" do - assert TwoFer.two_fer("Gilberto Barrosi") == "One for Gilberto Barros, one for me" - end - - @tag :pending - test "when the parameter is a number" do - assert_raise FunctionClauseError, fn -> - TwoFer.two_fer(10) - end - end - - @tag :pending - test "when the parameter is an atom" do - assert_raise FunctionClauseError, fn -> - TwoFer.two_fer(:bob) - end - end - - @tag :pending - test "when the parameter is a charlist" do - assert_raise FunctionClauseError, fn -> - refute TwoFer.two_fer('Jon Snow') - end - end -end From 35abd2cfa2c9c41fc3bc56a333dca40fc0a97ca5 Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Fri, 9 Jul 2021 11:37:30 +0200 Subject: [PATCH 10/16] Add moduledoc --- lib/elixir_analyzer/comment.ex | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/elixir_analyzer/comment.ex b/lib/elixir_analyzer/comment.ex index 5226fd7f..ded1ca5e 100644 --- a/lib/elixir_analyzer/comment.ex +++ b/lib/elixir_analyzer/comment.ex @@ -1,4 +1,9 @@ defmodule ElixirAnalyzer.Comment do + @moduledoc """ + Represents a single analysis comment + (see https://github.com/exercism/docs/blob/main/building/tooling/analyzers/interface.md#comments) + """ + defstruct status: :test, name: nil, comment: nil, type: nil, suppress_if: nil, params: nil @type t :: %__MODULE__{ From 2be48616c54dab9cd9921b3b28108404f3372e9f Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Fri, 9 Jul 2021 15:34:17 +0200 Subject: [PATCH 11/16] Add a check for snake_case module attribute names --- lib/elixir_analyzer/comment.ex | 3 +- lib/elixir_analyzer/constants.ex | 1 + lib/elixir_analyzer/exercise_test.ex | 3 + .../exercise_test/common_checks.ex | 16 ++++ .../common_checks/module_attribute_names.ex | 65 +++++++++++++ .../module_attribute_names_test.exs | 95 +++++++++++++++++++ .../exercise_test/common_checks_test.exs | 43 +++++++++ test/elixir_analyzer/exercise_test_test.exs | 2 + 8 files changed, 227 insertions(+), 1 deletion(-) create mode 100644 lib/elixir_analyzer/exercise_test/common_checks.ex create mode 100644 lib/elixir_analyzer/exercise_test/common_checks/module_attribute_names.ex create mode 100644 test/elixir_analyzer/exercise_test/common_checks/module_attribute_names_test.exs create mode 100644 test/elixir_analyzer/exercise_test/common_checks_test.exs diff --git a/lib/elixir_analyzer/comment.ex b/lib/elixir_analyzer/comment.ex index ded1ca5e..c674f68b 100644 --- a/lib/elixir_analyzer/comment.ex +++ b/lib/elixir_analyzer/comment.ex @@ -11,6 +11,7 @@ defmodule ElixirAnalyzer.Comment do comment: String.t(), type: :essential | :actionable | :informative | :celebratory, status: :skip | :test, - suppress_if: false | {String.t(), :pass | :fail} + suppress_if: false | {String.t(), :pass | :fail}, + params: map() } end diff --git a/lib/elixir_analyzer/constants.ex b/lib/elixir_analyzer/constants.ex index f6ea94e3..f17e83d6 100644 --- a/lib/elixir_analyzer/constants.ex +++ b/lib/elixir_analyzer/constants.ex @@ -18,6 +18,7 @@ defmodule ElixirAnalyzer.Constants do solution_use_moduledoc: "elixir.solution.use_module_doc", solution_use_specification: "elixir.solution.use_specification", solution_raise_fn_clause_error: "elixir.solution.raise_fn_clause_error", + solution_module_attribute_name_snake_case: "elixir.solution.module_attribute_name_snake_case", # Concept exercises diff --git a/lib/elixir_analyzer/exercise_test.ex b/lib/elixir_analyzer/exercise_test.ex index c0a7bca7..db4d0b66 100644 --- a/lib/elixir_analyzer/exercise_test.ex +++ b/lib/elixir_analyzer/exercise_test.ex @@ -3,6 +3,7 @@ defmodule ElixirAnalyzer.ExerciseTest do alias ElixirAnalyzer.ExerciseTest.Feature.Compiler, as: FeatureCompiler alias ElixirAnalyzer.ExerciseTest.AssertCall.Compiler, as: AssertCallCompiler + alias ElixirAnalyzer.ExerciseTest.CommonChecks alias ElixirAnalyzer.Submission alias ElixirAnalyzer.Constants @@ -45,10 +46,12 @@ defmodule ElixirAnalyzer.ExerciseTest do {:ok, code_ast} -> feature_results = unquote(feature_tests) |> filter_suppressed_results() assert_call_results = unquote(assert_call_tests) |> filter_suppressed_results() + common_checks_results = CommonChecks.run(code_ast, code_as_string) submission |> append_test_comments(feature_results) |> append_test_comments(assert_call_results) + |> append_test_comments(common_checks_results) {:error, e} -> append_analysis_failure(submission, e) diff --git a/lib/elixir_analyzer/exercise_test/common_checks.ex b/lib/elixir_analyzer/exercise_test/common_checks.ex new file mode 100644 index 00000000..2791f122 --- /dev/null +++ b/lib/elixir_analyzer/exercise_test/common_checks.ex @@ -0,0 +1,16 @@ +defmodule ElixirAnalyzer.ExerciseTest.CommonChecks do + @moduledoc """ + This module aggregates all common checks that should be run on every single solution. + """ + + alias ElixirAnalyzer.ExerciseTest.CommonChecks.ModuleAttributeNames + alias ElixirAnalyzer.Comment + + @spec run(Macro.t(), String.t()) :: [{:pass | :fail | :skip, %Comment{}}] + def run(code_ast, code_as_string) when is_binary(code_as_string) do + [ + ModuleAttributeNames.run(code_ast) + ] + |> List.flatten() + end +end diff --git a/lib/elixir_analyzer/exercise_test/common_checks/module_attribute_names.ex b/lib/elixir_analyzer/exercise_test/common_checks/module_attribute_names.ex new file mode 100644 index 00000000..08f58aec --- /dev/null +++ b/lib/elixir_analyzer/exercise_test/common_checks/module_attribute_names.ex @@ -0,0 +1,65 @@ +defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.ModuleAttributeNames do + @moduledoc """ + Reports the first module attribute with a name that's not snake_case. + + Doesn't report more if there are more. + A single comment should be enough for the student to know what to fix. + + Common check to be run on every single solution. + """ + + alias ElixirAnalyzer.Constants + alias ElixirAnalyzer.Comment + + @spec run(Macro.t()) :: [{:pass | :fail | :skip, %Comment{}}] + def run(ast) do + {_, names} = Macro.prewalk(ast, [], &traverse/2) + + if names == [] do + [] + else + wrong_name = + names + |> Enum.reverse() + |> hd() + |> to_string() + + correct_name = to_snake_case(wrong_name) + + [ + {:fail, + %Comment{ + type: :actionable, + comment: Constants.solution_module_attribute_name_snake_case(), + params: %{ + expected: correct_name, + actual: wrong_name + } + }} + ] + end + end + + defp traverse({:@, _meta, [{name, _meta2, _arguments}]} = ast, names) do + if snake_case?(name) do + {ast, names} + else + {ast, [name | names]} + end + end + + defp traverse(ast, names) do + {ast, names} + end + + defp snake_case?(name) do + # the code had to compile and pass all the tests to get to the analyzer + # so we can assume the name is otherwise valid + to_snake_case(name) == to_string(name) + end + + defp to_snake_case(name) do + # Macro.underscore is good enough because a module attribute name must be a valid Elixir identifier anyway + Macro.underscore(to_string(name)) + end +end diff --git a/test/elixir_analyzer/exercise_test/common_checks/module_attribute_names_test.exs b/test/elixir_analyzer/exercise_test/common_checks/module_attribute_names_test.exs new file mode 100644 index 00000000..e8df0635 --- /dev/null +++ b/test/elixir_analyzer/exercise_test/common_checks/module_attribute_names_test.exs @@ -0,0 +1,95 @@ +# credo:disable-for-this-file Credo.Check.Readability.ModuleAttributeNames + +defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.ModuleAttributeNamesTest do + use ExUnit.Case + alias ElixirAnalyzer.Comment + alias ElixirAnalyzer.Constants + alias ElixirAnalyzer.ExerciseTest.CommonChecks.ModuleAttributeNames + + test "returns empty list if there are no module attributes" do + code = + quote do + defmodule Factorial do + def calculate(1), do: 1 + + def calculate(n) do + n * calculate(n - 1) + end + end + end + + assert ModuleAttributeNames.run(code) == [] + end + + test "returns empty list if there are no module attributes with camelCase names" do + code = + quote do + defmodule Factorial do + @initial_value 1 + @spec calculate(integer) :: integer + def calculate(1), do: @initial_value + + def calculate(n) do + n * calculate(n - 1) + end + end + end + + assert ModuleAttributeNames.run(code) == [] + end + + test "returns an actionable comment with params" do + code = + quote do + defmodule Factorial do + @initialValue 1 + @spec calculate(integer) :: integer + def calculate(1), do: @initialValue + + def calculate(n) do + n * calculate(n - 1) + end + end + end + + assert ModuleAttributeNames.run(code) == [ + {:fail, + %Comment{ + type: :actionable, + comment: Constants.solution_module_attribute_name_snake_case(), + params: %{ + expected: "initial_value", + actual: "initialValue" + } + }} + ] + end + + test "only reports the first module attribute" do + code = + quote do + defmodule Factorial do + @somethingElseCamelCase + @initialValue 1 + @spec calculate(integer) :: integer + def calculate(1), do: @initialValue + + def calculate(n) do + n * calculate(n - 1) + end + end + end + + assert ModuleAttributeNames.run(code) == [ + {:fail, + %Comment{ + type: :actionable, + comment: Constants.solution_module_attribute_name_snake_case(), + params: %{ + expected: "something_else_camel_case", + actual: "somethingElseCamelCase" + } + }} + ] + end +end diff --git a/test/elixir_analyzer/exercise_test/common_checks_test.exs b/test/elixir_analyzer/exercise_test/common_checks_test.exs new file mode 100644 index 00000000..32de4c3e --- /dev/null +++ b/test/elixir_analyzer/exercise_test/common_checks_test.exs @@ -0,0 +1,43 @@ +# credo:disable-for-this-file Credo.Check.Readability.ModuleAttributeNames + +defmodule ElixirAnalyzer.ExerciseTestTest.Empty do + use ElixirAnalyzer.ExerciseTest +end + +defmodule ElixirAnalyzer.ExerciseTest.CommonChecksTest do + alias ElixirConstants.Constants + + use ElixirAnalyzer.ExerciseTestCase, + exercise_test_module: ElixirAnalyzer.ExerciseTestTest.Empty + + describe "module attribute names" do + test_exercise_analysis "doesn't report correct module attribute names", + comments_exclude: [Constants.solution_module_attribute_name_snake_case()] do + [ + defmodule SomeModule do + @foo_bar + end, + defmodule SomeModule do + @foo_bar1 + @foo_bar2 + end, + defmodule SomeModule do + @foo_bar1 5 + @foo_bar2 2 + end + ] + end + + test_exercise_analysis "reports a module attribute that doesn't use snake_case", + comments_include: [Constants.solution_module_attribute_name_snake_case()] do + [ + defmodule SomeModule do + @fooBar + end, + defmodule SomeModule do + @someValue 3 + end + ] + end + end +end diff --git a/test/elixir_analyzer/exercise_test_test.exs b/test/elixir_analyzer/exercise_test_test.exs index ab43f572..c5679df1 100644 --- a/test/elixir_analyzer/exercise_test_test.exs +++ b/test/elixir_analyzer/exercise_test_test.exs @@ -47,6 +47,8 @@ defmodule ElixirAnalyzer.ExerciseTestTest.SameComment do end defmodule ElixirAnalyzer.ExerciseTestTest do + alias ElixirConstants.Constants + use ElixirAnalyzer.ExerciseTestCase, exercise_test_module: ElixirAnalyzer.ExerciseTestTest.SameComment From f4562e2b48263373b847c7b9144e2e7d96d084bf Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Fri, 9 Jul 2021 15:46:30 +0200 Subject: [PATCH 12/16] Run common checks when no dedicated module --- README.md | 4 ++-- lib/elixir_analyzer.ex | 18 ++++++++---------- lib/elixir_analyzer/test_suite/default.ex | 10 ++++++++++ test/elixir_analyzer_test.exs | 11 +++++++---- .../two_fer/imperfect_solution/lib/two_fer.ex | 2 ++ 5 files changed, 29 insertions(+), 16 deletions(-) create mode 100644 lib/elixir_analyzer/test_suite/default.ex diff --git a/README.md b/README.md index fee2b24c..614a8ad3 100644 --- a/README.md +++ b/README.md @@ -57,7 +57,7 @@ As a demonstration, once iex loads with `iex -S mix` you can type `ElixirAnalyze ### `ElixirAnalyzer` - The is the main application module. A function call to `start_analyze/3` begins the analysis (either through IEX or the CLI escript [if generated]). -- A configuation in `config/config.exs` holds data for each exercise supported +- A configuration in `config/config.exs` holds data for each exercise supported ```elixir config :elixir_analyzer, @@ -110,7 +110,7 @@ Contains macro to generate a function returning the comment path on the `exercis ### `ElixirAnalyzer.CLI` -This module is a module for the CLI escript to parse the command line arguements and start the processing +This module is a module for the CLI escript to parse the command line arguments and start the processing ### `ElixirAnalyzer.ExerciseTest.________` diff --git a/lib/elixir_analyzer.ex b/lib/elixir_analyzer.ex index e3ac545c..d548bf40 100644 --- a/lib/elixir_analyzer.ex +++ b/lib/elixir_analyzer.ex @@ -134,15 +134,6 @@ defmodule ElixirAnalyzer do analysis_module: analysis_module } rescue - e in ArgumentError -> - Logger.warning("TestSuite halted, ArgumentError", error_message: e.message) - - submission - |> Submission.halt() - |> Submission.set_halt_reason( - "Analysis skipped, no analysis suite exists for this exercise" - ) - e in File.Error -> Logger.warning("Unable to decode 'config.json'", error_message: e.message) @@ -156,6 +147,13 @@ defmodule ElixirAnalyzer do submission |> Submission.halt() |> Submission.set_halt_reason("Analysis skipped, not able to decode solution config.") + + e -> + Logger.warning("TestSuite halted, #{e.__struct__}", error_message: e.message) + + submission + |> Submission.halt() + |> Submission.set_halt_reason("Analysis skipped, unexpected error #{e.__struct__}") end end @@ -168,7 +166,7 @@ defmodule ElixirAnalyzer do code_path = Path.dirname(full_code_path) code_file = Path.basename(full_code_path) - {code_path, code_file, exercise_config[:analyzer_module]} + {code_path, code_file, exercise_config[:analyzer_module] || ElixirAnalyzer.TestSuite.Default} end # Else, use passed in params to analyze diff --git a/lib/elixir_analyzer/test_suite/default.ex b/lib/elixir_analyzer/test_suite/default.ex new file mode 100644 index 00000000..c50e1c95 --- /dev/null +++ b/lib/elixir_analyzer/test_suite/default.ex @@ -0,0 +1,10 @@ +defmodule ElixirAnalyzer.TestSuite.Default do + @moduledoc """ + This is the default exercise analyzer extension module. + + It will be run for any exercise submission that doesn't have its own extension module. + It's empty, which means it will only run the common checks. + """ + + use ElixirAnalyzer.ExerciseTest +end diff --git a/test/elixir_analyzer_test.exs b/test/elixir_analyzer_test.exs index 6e41ca9a..baa09e4c 100644 --- a/test/elixir_analyzer_test.exs +++ b/test/elixir_analyzer_test.exs @@ -29,7 +29,7 @@ defmodule ElixirAnalyzerTest do analyzed_exercise = ElixirAnalyzer.analyze_exercise(exercise, path, path, @options) expected_output = """ - {\"comments\":[{\"comment\":\"elixir.solution.use_module_doc\",\"type\":\"informative\"},{\"comment\":\"elixir.solution.raise_fn_clause_error\",\"type\":\"actionable\"},{\"comment\":\"elixir.two-fer.use_of_function_header\",\"type\":\"actionable\"},{\"comment\":\"elixir.solution.use_specification\",\"type\":\"actionable\"}],\"summary\":\"Check the comments for some code suggestions. 📣\"} + {\"comments\":[{\"comment\":\"elixir.solution.use_module_doc\",\"type\":\"informative\"},{\"comment\":\"elixir.solution.raise_fn_clause_error\",\"type\":\"actionable\"},{\"comment\":\"elixir.two-fer.use_of_function_header\",\"type\":\"actionable\"},{\"comment\":\"elixir.solution.use_specification\",\"type\":\"actionable\"},{\"comment\":\"elixir.solution.module_attribute_name_snake_case\",\"params\":{\"actual\":\"someUnusedModuleAttribute\",\"expected\":\"some_unused_module_attribute\"},\"type\":\"actionable\"}],\"summary\":\"Check the comments for some code suggestions. 📣\"} """ assert Submission.to_json(analyzed_exercise) == String.trim(expected_output) @@ -63,15 +63,15 @@ defmodule ElixirAnalyzerTest do assert Submission.to_json(analyzed_exercise) == String.trim(expected_output) end - test "solution for an exercise with no analyzer module" do + test "solution for an exercise with no analyzer module uses the default module" do exercise = "not-a-real-exercise" - path = "./test_data/two_fer/error_solution/" + path = "./test_data/two_fer/imperfect_solution/" capture_log(fn -> analyzed_exercise = ElixirAnalyzer.analyze_exercise(exercise, path, path, @options) expected_output = """ - {\"comments\":[],\"summary\":\"Analysis was halted. Analysis skipped, no analysis suite exists for this exercise\"} + {\"comments\":[{\"comment\":\"elixir.solution.module_attribute_name_snake_case\",\"params\":{\"actual\":\"someUnusedModuleAttribute\",\"expected\":\"some_unused_module_attribute\"},\"type\":\"actionable\"}],\"summary\":\"Check the comments for some code suggestions. 📣\"} """ assert Submission.to_json(analyzed_exercise) == String.trim(expected_output) @@ -97,6 +97,9 @@ defmodule ElixirAnalyzerTest do unused_available_test_suites = all_available_test_suites -- test_suites_referenced_in_config + unused_available_test_suites = + unused_available_test_suites -- [ElixirAnalyzer.TestSuite.Default] + unavailable_test_suites_referenced_in_config = test_suites_referenced_in_config -- all_available_test_suites diff --git a/test_data/two_fer/imperfect_solution/lib/two_fer.ex b/test_data/two_fer/imperfect_solution/lib/two_fer.ex index bd4dfe6a..d96a8770 100644 --- a/test_data/two_fer/imperfect_solution/lib/two_fer.ex +++ b/test_data/two_fer/imperfect_solution/lib/two_fer.ex @@ -1,4 +1,6 @@ defmodule TwoFer do + @someUnusedModuleAttribute 1 + @doc """ Two-fer or 2-fer is short for two for one. One for you and one for me. """ From 2b5b3a7a86c54b8b984ab8b181ee7e7aac81c8eb Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Fri, 9 Jul 2021 16:22:32 +0200 Subject: [PATCH 13/16] Add a check for snake_case function names --- lib/elixir_analyzer/constants.ex | 1 + .../exercise_test/common_checks.ex | 2 + .../common_checks/function_names.ex | 67 +++++++ .../common_checks/function_names_test.exs | 177 ++++++++++++++++++ .../exercise_test/common_checks_test.exs | 87 +++++++++ 5 files changed, 334 insertions(+) create mode 100644 lib/elixir_analyzer/exercise_test/common_checks/function_names.ex create mode 100644 test/elixir_analyzer/exercise_test/common_checks/function_names_test.exs diff --git a/lib/elixir_analyzer/constants.ex b/lib/elixir_analyzer/constants.ex index f17e83d6..c3ee4873 100644 --- a/lib/elixir_analyzer/constants.ex +++ b/lib/elixir_analyzer/constants.ex @@ -19,6 +19,7 @@ defmodule ElixirAnalyzer.Constants do solution_use_specification: "elixir.solution.use_specification", solution_raise_fn_clause_error: "elixir.solution.raise_fn_clause_error", solution_module_attribute_name_snake_case: "elixir.solution.module_attribute_name_snake_case", + solution_function_name_snake_case: "elixir.solution.function_name_snake_case", # Concept exercises diff --git a/lib/elixir_analyzer/exercise_test/common_checks.ex b/lib/elixir_analyzer/exercise_test/common_checks.ex index 2791f122..01017b8a 100644 --- a/lib/elixir_analyzer/exercise_test/common_checks.ex +++ b/lib/elixir_analyzer/exercise_test/common_checks.ex @@ -3,12 +3,14 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecks do This module aggregates all common checks that should be run on every single solution. """ + alias ElixirAnalyzer.ExerciseTest.CommonChecks.FunctionNames alias ElixirAnalyzer.ExerciseTest.CommonChecks.ModuleAttributeNames alias ElixirAnalyzer.Comment @spec run(Macro.t(), String.t()) :: [{:pass | :fail | :skip, %Comment{}}] def run(code_ast, code_as_string) when is_binary(code_as_string) do [ + FunctionNames.run(code_ast), ModuleAttributeNames.run(code_ast) ] |> List.flatten() diff --git a/lib/elixir_analyzer/exercise_test/common_checks/function_names.ex b/lib/elixir_analyzer/exercise_test/common_checks/function_names.ex new file mode 100644 index 00000000..6aed6d2b --- /dev/null +++ b/lib/elixir_analyzer/exercise_test/common_checks/function_names.ex @@ -0,0 +1,67 @@ +defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.FunctionNames do + @moduledoc """ + Reports the first function/macro/guard with a name that's not snake_case. + + Doesn't report more if there are more. + A single comment should be enough for the student to know what to fix. + + Common check to be run on every single solution. + """ + + alias ElixirAnalyzer.Constants + alias ElixirAnalyzer.Comment + + @def_ops [:def, :defp, :defmacro, :defmacrop, :defguard, :defguardp] + + @spec run(Macro.t()) :: [{:pass | :fail | :skip, %Comment{}}] + def run(ast) do + {_, names} = Macro.prewalk(ast, [], &traverse/2) + + if names == [] do + [] + else + wrong_name = + names + |> Enum.reverse() + |> hd() + |> to_string() + + correct_name = to_snake_case(wrong_name) + + [ + {:fail, + %Comment{ + type: :actionable, + comment: Constants.solution_function_name_snake_case(), + params: %{ + expected: correct_name, + actual: wrong_name + } + }} + ] + end + end + + defp traverse({op, _meta, [{name, _meta2, _arguments} | _]} = ast, names) when op in @def_ops do + if snake_case?(name) do + {ast, names} + else + {ast, [name | names]} + end + end + + defp traverse(ast, names) do + {ast, names} + end + + defp snake_case?(name) do + # the code had to compile and pass all the tests to get to the analyzer + # so we can assume the name is otherwise valid + to_snake_case(name) == to_string(name) + end + + defp to_snake_case(name) do + # Macro.underscore is good enough because a module attribute name must be a valid Elixir identifier anyway + Macro.underscore(to_string(name)) + end +end diff --git a/test/elixir_analyzer/exercise_test/common_checks/function_names_test.exs b/test/elixir_analyzer/exercise_test/common_checks/function_names_test.exs new file mode 100644 index 00000000..6283ef0c --- /dev/null +++ b/test/elixir_analyzer/exercise_test/common_checks/function_names_test.exs @@ -0,0 +1,177 @@ +# credo:disable-for-this-file Credo.Check.Readability.FunctionNames + +defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.FunctionNamesTest do + use ExUnit.Case + alias ElixirAnalyzer.Comment + alias ElixirAnalyzer.Constants + alias ElixirAnalyzer.ExerciseTest.CommonChecks.FunctionNames + + test "returns empty list if there are functions, macros, or guards" do + code = + quote do + defmodule User do + defstruct(:name, :email) + end + end + + assert FunctionNames.run(code) == [] + end + + test "doesn't crash when a variable is named def" do + code = + quote do + defmodule User do + def name(user) do + def = user.first_name + defmacro = user.last_name + def <> " " <> defmacro + end + end + end + + assert FunctionNames.run(code) == [] + end + + test "returns empty list if all names are correct" do + code = + quote do + defmodule User do + def first_name(user), do: user.first_name + defp registered_in_last_quarter?(user), do: true + defmacro validate_user!, do: true + defmacrop do_validate_user, do: true + defguard(is_user, do: true) + defguardp(is_registered_user, do: true) + end + end + + assert FunctionNames.run(code) == [] + end + + test "returns an actionable comment with params" do + code = + quote do + defmodule User do + def firstName(user), do: user.first_name + end + end + + assert FunctionNames.run(code) == [ + {:fail, + %Comment{ + type: :actionable, + comment: Constants.solution_function_name_snake_case(), + params: %{ + expected: "first_name", + actual: "firstName" + } + }} + ] + end + + test "works for def, defp, defmacro, defmacrop, defguard, and defguardp" do + get_result = fn expected, actual -> + [ + {:fail, + %Comment{ + type: :actionable, + comment: Constants.solution_function_name_snake_case(), + params: %{ + expected: expected, + actual: actual + } + }} + ] + end + + code = + quote do + defmodule User do + defp registeredInLastQuarter?(user), do: true + end + end + + assert FunctionNames.run(code) == + get_result.("registered_in_last_quarter?", "registeredInLastQuarter?") + + code = + quote do + defmodule User do + defmacro validateUser!, do: true + end + end + + assert FunctionNames.run(code) == get_result.("validate_user!", "validateUser!") + + code = + quote do + defmodule User do + defmacrop doValidateUser, do: true + end + end + + assert FunctionNames.run(code) == get_result.("do_validate_user", "doValidateUser") + + code = + quote do + defmodule User do + defguard(isUser, do: true) + end + end + + assert FunctionNames.run(code) == get_result.("is_user", "isUser") + + code = + quote do + defmodule User do + defguardp(isRegisteredUser, do: true) + end + end + + assert FunctionNames.run(code) == get_result.("is_registered_user", "isRegisteredUser") + end + + test "only reports the first wrong name" do + code = + quote do + defmodule User do + def first_name(user), do: getFirstName(user) + defguard(isRegisteredUser(user), do: true) + defp getFirstName(user), do: user.first_name + end + end + + assert FunctionNames.run(code) == [ + {:fail, + %Comment{ + type: :actionable, + comment: Constants.solution_function_name_snake_case(), + params: %{ + expected: "is_registered_user", + actual: "isRegisteredUser" + } + }} + ] + end + + test "can handle function heads" do + code = + quote do + defmodule User do + def getName(user, fallback \\ "Doe") + end + end + + assert FunctionNames.run(code) == [ + {:fail, + %Comment{ + type: :actionable, + comment: Constants.solution_function_name_snake_case(), + params: %{ + expected: "get_name", + actual: "getName" + } + }} + ] + end +end diff --git a/test/elixir_analyzer/exercise_test/common_checks_test.exs b/test/elixir_analyzer/exercise_test/common_checks_test.exs index 32de4c3e..ce449d2b 100644 --- a/test/elixir_analyzer/exercise_test/common_checks_test.exs +++ b/test/elixir_analyzer/exercise_test/common_checks_test.exs @@ -1,4 +1,5 @@ # credo:disable-for-this-file Credo.Check.Readability.ModuleAttributeNames +# credo:disable-for-this-file Credo.Check.Readability.FunctionNames defmodule ElixirAnalyzer.ExerciseTestTest.Empty do use ElixirAnalyzer.ExerciseTest @@ -10,6 +11,92 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecksTest do use ElixirAnalyzer.ExerciseTestCase, exercise_test_module: ElixirAnalyzer.ExerciseTestTest.Empty + describe "function, guard, macro names" do + test_exercise_analysis "doesn't report correct names", + comments_exclude: [Constants.solution_function_name_snake_case()] do + [ + defmodule SomeModule do + def foo_bar, do: 1 + + def foo_bar_baz do + foo_bar + 8 + end + end, + defmodule SomeModule do + defp foo_bar, do: 1 + + defp foo_bar_baz do + foo_bar + 8 + end + end, + defmodule SomeModule do + defmacro foo_bar, do: 1 + + defmacro foo_bar_baz do + foo_bar + 8 + end + end, + defmodule SomeModule do + defmacrop foo_bar, do: 1 + + defmacrop foo_bar_baz do + foo_bar + 8 + end + end, + defmodule SomeModule do + defguard(foo_bar, do: 1) + + defguard foo_bar_baz do + foo_bar + 8 + end + end, + defmodule SomeModule do + defguardp(foo_bar, do: 1) + + defguardp foo_bar_baz do + foo_bar + 8 + end + end + ] + end + + test_exercise_analysis "reports incorrect names", + comments_include: [Constants.solution_function_name_snake_case()] do + [ + defmodule SomeModule do + def fooBarBaz do + foo_bar + 8 + end + end, + defmodule SomeModule do + defp fooBarBaz do + foo_bar + 8 + end + end, + defmodule SomeModule do + defmacro fooBarBaz do + foo_bar + 8 + end + end, + defmodule SomeModule do + defmacrop fooBarBaz do + foo_bar + 8 + end + end, + defmodule SomeModule do + defguard fooBarBaz do + foo_bar + 8 + end + end, + defmodule SomeModule do + defguardp fooBarBaz do + foo_bar + 8 + end + end + ] + end + end + describe "module attribute names" do test_exercise_analysis "doesn't report correct module attribute names", comments_exclude: [Constants.solution_module_attribute_name_snake_case()] do From 6e9a93b98af3449b0e7fe8b4c9328019d9d4e9c8 Mon Sep 17 00:00:00 2001 From: Tim Austin Date: Fri, 9 Jul 2021 20:00:05 -0600 Subject: [PATCH 14/16] Update lib/elixir_analyzer/test_suite/default.ex --- lib/elixir_analyzer/test_suite/default.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elixir_analyzer/test_suite/default.ex b/lib/elixir_analyzer/test_suite/default.ex index c50e1c95..eb5a7186 100644 --- a/lib/elixir_analyzer/test_suite/default.ex +++ b/lib/elixir_analyzer/test_suite/default.ex @@ -3,7 +3,7 @@ defmodule ElixirAnalyzer.TestSuite.Default do This is the default exercise analyzer extension module. It will be run for any exercise submission that doesn't have its own extension module. - It's empty, which means it will only run the common checks. + It's intentionally empty, which means it will only run the common checks. """ use ElixirAnalyzer.ExerciseTest From fb22547e8fc06a49d8bb297759e297b1e47f848d Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Sat, 10 Jul 2021 08:53:33 +0200 Subject: [PATCH 15/16] Update lib/elixir_analyzer/exercise_test/common_checks/function_names.ex Co-authored-by: Tim Austin --- .../common_checks/function_names.ex | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/lib/elixir_analyzer/exercise_test/common_checks/function_names.ex b/lib/elixir_analyzer/exercise_test/common_checks/function_names.ex index 6aed6d2b..449165f7 100644 --- a/lib/elixir_analyzer/exercise_test/common_checks/function_names.ex +++ b/lib/elixir_analyzer/exercise_test/common_checks/function_names.ex @@ -16,16 +16,10 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.FunctionNames do @spec run(Macro.t()) :: [{:pass | :fail | :skip, %Comment{}}] def run(ast) do {_, names} = Macro.prewalk(ast, [], &traverse/2) - - if names == [] do - [] - else - wrong_name = - names - |> Enum.reverse() - |> hd() - |> to_string() - + wrong_name = List.last(names) + + if wrong_name do + wrong_name = to_string(wrong_name) correct_name = to_snake_case(wrong_name) [ @@ -39,6 +33,8 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.FunctionNames do } }} ] + else + [] end end From e39b2cbe3f0565ff0b8f5ff6e3ccbf4826337479 Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Sat, 10 Jul 2021 08:54:45 +0200 Subject: [PATCH 16/16] Simplify further --- .../exercise_test/common_checks/function_names.ex | 2 +- .../common_checks/module_attribute_names.ex | 14 +++++--------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/lib/elixir_analyzer/exercise_test/common_checks/function_names.ex b/lib/elixir_analyzer/exercise_test/common_checks/function_names.ex index 449165f7..15a37616 100644 --- a/lib/elixir_analyzer/exercise_test/common_checks/function_names.ex +++ b/lib/elixir_analyzer/exercise_test/common_checks/function_names.ex @@ -17,7 +17,7 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.FunctionNames do def run(ast) do {_, names} = Macro.prewalk(ast, [], &traverse/2) wrong_name = List.last(names) - + if wrong_name do wrong_name = to_string(wrong_name) correct_name = to_snake_case(wrong_name) diff --git a/lib/elixir_analyzer/exercise_test/common_checks/module_attribute_names.ex b/lib/elixir_analyzer/exercise_test/common_checks/module_attribute_names.ex index 08f58aec..3cb7904a 100644 --- a/lib/elixir_analyzer/exercise_test/common_checks/module_attribute_names.ex +++ b/lib/elixir_analyzer/exercise_test/common_checks/module_attribute_names.ex @@ -14,16 +14,10 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.ModuleAttributeNames do @spec run(Macro.t()) :: [{:pass | :fail | :skip, %Comment{}}] def run(ast) do {_, names} = Macro.prewalk(ast, [], &traverse/2) + wrong_name = List.last(names) - if names == [] do - [] - else - wrong_name = - names - |> Enum.reverse() - |> hd() - |> to_string() - + if wrong_name do + wrong_name = to_string(wrong_name) correct_name = to_snake_case(wrong_name) [ @@ -37,6 +31,8 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.ModuleAttributeNames do } }} ] + else + [] end end