-
-
Notifications
You must be signed in to change notification settings - Fork 30
100% test coverage #246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
100% test coverage #246
Changes from all commits
05ac98f
c7464a5
f537273
541319f
367afaa
d6d38e6
2a928a6
c5ff3d9
d6fc181
08dc424
74063df
7284304
d15d73d
ff039e1
65bbe0d
ae496ca
11c53f9
db5858f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,18 +29,14 @@ defmodule ElixirAnalyzer do | |
|
||
* `exercise` is which exercise is submitted to determine proper analysis | ||
|
||
* `path` is the path (ending with a '/') to the submitted solution | ||
* `input_path` is the path to the submitted solution | ||
|
||
* `output_path` is the path to the output folder | ||
|
||
* `opts` is a Keyword List of options, see **options** | ||
|
||
## Options | ||
|
||
* `:exercise` - name of the exercise, defaults to the `exercise` parameter | ||
|
||
* `:path` - path to the submitted solution, defaults to the `path` parameter | ||
|
||
* `:output_path` - path to write file output, defaults to the `path` parameter | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did not understand these options here, all they do is overwrite the required arguments. Those options were passed from the CLI module. I got rid of them. |
||
|
||
* `:output_file`, - specifies the name of the output_file, defaults to | ||
`@output_file` (`analysis.json`) | ||
|
||
|
@@ -52,8 +48,6 @@ defmodule ElixirAnalyzer do | |
|
||
* `:puts_summary` - boolean flag if an analysis should print the summary of the | ||
analysis to stdio, defaults to `true` | ||
|
||
Any arbitrary keyword-value pair can be passed to `analyze_exercise/3` and these options may be used the other consuming code. | ||
""" | ||
@spec analyze_exercise(String.t(), String.t(), String.t(), keyword()) :: Submission.t() | ||
def analyze_exercise(exercise, input_path, output_path, opts \\ []) do | ||
|
@@ -147,14 +141,14 @@ defmodule ElixirAnalyzer do | |
} | ||
rescue | ||
e in File.Error -> | ||
Logger.warning("Unable to decode 'config.json'", error_message: e.message) | ||
Logger.warning("Unable to read config file #{e.path}", error_message: e.reason) | ||
|
||
submission | ||
|> Submission.halt() | ||
|> Submission.set_halt_reason("Analysis skipped, not able to read solution config.") | ||
|
||
e in Jason.DecodeError -> | ||
Logger.warning("Unable to decode 'config.json'", error_message: e.message) | ||
Logger.warning("Unable to decode 'config.json'", data: e.data) | ||
|
||
submission | ||
|> Submission.halt() | ||
|
@@ -256,7 +250,7 @@ defmodule ElixirAnalyzer do | |
|
||
submission = | ||
submission | ||
|> submission.analysis_module.analyze(submission.source) | ||
|> submission.analysis_module.analyze() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice getting rid of a duplicated argument 👍 |
||
|> Submission.set_analyzed(true) | ||
|
||
Logger.info("Analyzing code complete") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,22 +6,19 @@ defmodule ElixirAnalyzer.CLI do | |
@usage """ | ||
Usage: | ||
|
||
$ elixir_analyzer <exercise-name> <input path> <output path> [options] | ||
$ elixir_analyzer <exercise-name> <input path> <output path> [options] | ||
|
||
You may also pass the following options: | ||
--skip-analysis flag skips running the static analysis | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand the purpose of skipping analysis except for debugging, so I got rid of that option too. I think it still exists somewhere in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Do you mean the ability to skip a single check? Because I don't see anything related to skipping analysis as a whole anywhere else 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I suppose that's what I meant. There's probably no relation beyond the name. |
||
--output-file <filename> | ||
jiegillet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
You may also test only individual files : | ||
(assuming analyzer tests are compiled for the named module) | ||
|
||
$ exercism_analyzer --analyze-file <full-path-to-.ex>:<module-name> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was an option in the CLI, but could not actually be passed to the analyzer, so I got rid of it |
||
--help see this message | ||
--output-file <filename> output file name (default: analysis.json) | ||
--no-write-results doesn't write to JSON file | ||
--no-puts-summary doesn't print summary to stdio | ||
""" | ||
|
||
@options [ | ||
{{:skip_analyze, :boolean}, false}, | ||
{{:output_file, :string}, "analysis.json"}, | ||
{{:analyze_file, :string}, nil}, | ||
{{:write_results, :boolean}, true}, | ||
{{:puts_summary, :boolean}, true}, | ||
{{:help, :boolean}, false} | ||
] | ||
|
||
|
@@ -30,45 +27,27 @@ defmodule ElixirAnalyzer.CLI do | |
args |> parse_args() |> process() | ||
end | ||
|
||
def parse_args(args) do | ||
options = %{ | ||
:output_file => "analysis.json" | ||
} | ||
defp parse_args(args) do | ||
default_ops = for({{key, _}, val} <- @options, do: {key, val}, into: %{}) | ||
|
||
cmd_opts = | ||
OptionParser.parse(args, | ||
strict: for({o, _} <- @options, do: o) | ||
) | ||
cmd_opts = OptionParser.parse(args, strict: for({o, _} <- @options, do: o)) | ||
|
||
case cmd_opts do | ||
{[help: true], _, _} -> | ||
:help | ||
|
||
{[analyze_file: target], _, _} -> | ||
[full_path, module] = String.split(target, ":", trim: true) | ||
path = Path.dirname(full_path) | ||
file = Path.basename(full_path) | ||
{Enum.into([module: module, file: file], options), "undefined", path} | ||
|
||
{opts, [exercise, input_path, output_path], _} -> | ||
{Enum.into(opts, options), exercise, input_path, output_path} | ||
{Enum.into(opts, default_ops), exercise, input_path, output_path} | ||
end | ||
rescue | ||
_ -> :help | ||
end | ||
|
||
def process(:help), do: IO.puts(@usage) | ||
defp process(:help), do: IO.puts(@usage) | ||
|
||
def process({options, exercise, input_path, output_path}) do | ||
opts = get_default_options(options) | ||
ElixirAnalyzer.analyze_exercise(exercise, input_path, output_path, opts) | ||
end | ||
defp process({options, exercise, input_path, output_path}) do | ||
opts = Map.to_list(options) | ||
|
||
defp get_default_options(options) do | ||
@options | ||
|> Enum.reduce(options, fn {{option, _}, default}, acc -> | ||
Map.put_new(acc, option, default) | ||
end) | ||
|> Map.to_list() | ||
ElixirAnalyzer.analyze_exercise(exercise, input_path, output_path, opts) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -54,13 +54,13 @@ defmodule ElixirAnalyzer.ExerciseTest.CommonChecks.FunctionCapture do | |||||||||||||
depth = depth - 1 | ||||||||||||||
|
||||||||||||||
functions = | ||||||||||||||
if depth == 0 and wrong_use? and actual_function?(name) and name not in @exceptions do | ||||||||||||||
if depth <= 0 and wrong_use? and actual_function?(name) and name not in @exceptions do | ||||||||||||||
[{:&, name, length(args)} | functions] | ||||||||||||||
else | ||||||||||||||
functions | ||||||||||||||
end | ||||||||||||||
|
||||||||||||||
{node, %{capture_depth: depth - 1, functions: functions}} | ||||||||||||||
{node, %{capture_depth: depth, functions: functions}} | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How did you discover this bug? When I bring it back, no tests fail, so it wasn't by writing tests 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry about that! I took me a while to remember. elixir-analyzer/test/elixir_analyzer/exercise_test/common_checks/function_capture_test.exs Lines 53 to 58 in 926d69d
At some point I was checking if I've added a failing test now. Let that be a valuable lesson of never fixing bugs without context. |
||||||||||||||
end | ||||||||||||||
|
||||||||||||||
# fn -> foo end | ||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,10 +53,16 @@ defmodule ElixirAnalyzer.ExerciseTest.Feature do | |
feature_data = %{feature_data | meta: Map.to_list(feature_data.meta)} | ||
feature_data = Map.to_list(feature_data) | ||
|
||
unless Keyword.has_key?(feature_data, :comment) do | ||
raise "Comment must be defined for each feature test" | ||
end | ||
Comment on lines
+56
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awesome 🎉 I didn't notice before that this one macro is missing the comment check |
||
|
||
quote do | ||
# Check if the feature is unique | ||
case Enum.filter(@feature_tests, fn {_data, forms} -> | ||
forms == unquote(Macro.escape(feature_forms)) | ||
case Enum.filter(@feature_tests, fn {data, forms} -> | ||
{Keyword.get(data, :find), Keyword.get(data, :depth), forms} == | ||
{Keyword.get(unquote(feature_data), :find), | ||
Keyword.get(unquote(feature_data), :depth), unquote(Macro.escape(feature_forms))} | ||
end) do | ||
[{data, _forms} | _] -> | ||
raise FeatureError, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,15 +68,6 @@ defmodule ElixirAnalyzer.QuoteUtil do | |
end) | ||
end | ||
|
||
@doc """ | ||
Performs a depth-first, pre-order traversal of quoted expressions. | ||
With depth provided to a function | ||
""" | ||
@spec prewalk(Macro.t(), (Macro.t(), non_neg_integer -> Macro.t())) :: Macro.t() | ||
def prewalk(ast, fun) when is_function(fun, 2) do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was a bit hesitant to get rid of this. It seems like it could be useful, but it's not actually used anywhere. I'm assuming at this point we won't need to add another complex analysis tool. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No mercy for dead code! Even if it looks potentially useful, we will be able to recreate it when we need it. Making the code base easier to understand by removing trash is the better option every time, IMO. |
||
elem(prewalk(ast, nil, fn x, nil, d -> {fun.(x, d), nil} end), 0) | ||
end | ||
|
||
@doc """ | ||
Performs a depth-first, pre-order traversal of quoted expressions | ||
using an accumulator. | ||
|
@@ -86,22 +77,4 @@ defmodule ElixirAnalyzer.QuoteUtil do | |
def prewalk(ast, acc, fun) when is_function(fun, 3) do | ||
traverse_with_depth(ast, acc, fun, fn x, a, _d -> {x, a} end) | ||
end | ||
|
||
@doc """ | ||
Performs a depth-first, post-order traversal of quoted expressions. | ||
""" | ||
@spec postwalk(Macro.t(), (Macro.t(), non_neg_integer -> Macro.t())) :: Macro.t() | ||
def postwalk(ast, fun) when is_function(fun, 2) do | ||
elem(postwalk(ast, nil, fn x, nil, d -> {fun.(x, d), nil} end), 0) | ||
end | ||
|
||
@doc """ | ||
Performs a depth-first, post-order traversal of quoted expressions | ||
using an accumulator. | ||
""" | ||
@spec postwalk(Macro.t(), any, (Macro.t(), any, non_neg_integer -> {Macro.t(), any})) :: | ||
{Macro.t(), any} | ||
def postwalk(ast, acc, fun) when is_function(fun, 3) do | ||
traverse_with_depth(ast, acc, fn x, a, _d -> {x, a} end, fun) | ||
end | ||
end |
Uh oh!
There was an error while loading. Please reload this page.