diff --git a/app/.credo.exs b/app/.credo.exs index 88f64e3..7ee56f7 100644 --- a/app/.credo.exs +++ b/app/.credo.exs @@ -4,6 +4,13 @@ # If you find anything wrong or unclear in this file, please report an # issue on GitHub: https://github.com/rrrene/credo/issues # +# The gate for this app is `mix credo --strict` on the command line (the +# `credo` target in Makefile, a prerequisite of `check`). The command line, not +# this file, decides: `--strict` overrides the inert `strict: false` key below. +# Editing `strict` here to silence a finding would change nothing - fix the +# finding, or write down why the check is wrong for this codebase (see +# `disabled:`). +# %{ # # You can have as many configs as you like in the `configs:` field. @@ -126,7 +133,6 @@ # {Credo.Check.Refactor.Apply, []}, {Credo.Check.Refactor.CondStatements, []}, - {Credo.Check.Refactor.CyclomaticComplexity, []}, {Credo.Check.Refactor.FilterCount, []}, {Credo.Check.Refactor.FilterFilter, []}, {Credo.Check.Refactor.FunctionArity, []}, @@ -174,6 +180,18 @@ {Firehose.Checks.NoConnShadowing, []} ], disabled: [ + # + # Our one deliberate deviation from credo's defaults. CRAP (`ex_crap`, + # threshold 30, wired into the Lens pass in Makefile) is this app's + # complexity sensor, and it scores complexity against real coverage: a + # wide `case` that the tests exercise is not a problem - that was the + # itch behind ExCrap. A second, coverage-blind complexity sensor would + # just argue with the first, so complexity questions are settled by + # the CRAP target. Re-enabling this check without retiring the CRAP + # target is a policy change, not a cleanup. + # + {Credo.Check.Refactor.CyclomaticComplexity, []}, + # # Checks scheduled for next check update (opt-in for now) {Credo.Check.Refactor.UtcNowTruncate, []}, diff --git a/app/test/credo_config_test.exs b/app/test/credo_config_test.exs new file mode 100644 index 0000000..ed36023 --- /dev/null +++ b/app/test/credo_config_test.exs @@ -0,0 +1,80 @@ +defmodule Firehose.CredoConfigTest do + @moduledoc """ + Pins the *policy* in `app/.credo.exs`, not just the file's presence. + + The gate is `mix credo --strict` (the `credo` target in `app/Makefile`, a + prerequisite of `check`), so the config decides what every future commit has + to satisfy. Plain `ExUnit.Case`: this reads and evaluates a file, it does not + touch the database or boot the app's supervision tree. + + The assertions are deliberately coarse - "this check sits in that list", + "the disable says why", "the custom check is still loaded" - so they survive + credo's own boilerplate churn but not a policy change. + """ + use ExUnit.Case, async: true + + @config_path ".credo.exs" + @complexity_check "Credo.Check.Refactor.CyclomaticComplexity" + @custom_check "Firehose.Checks.NoConnShadowing" + @custom_check_load_path "lib_dev/firehose/checks/" + + test "cyclomatic complexity is disabled in favour of CRAP" do + checks = default_config()[:checks] + + assert @complexity_check in check_modules(checks[:disabled]) + refute @complexity_check in check_modules(checks[:enabled]) + end + + test "the disable carries a rationale comment" do + rationale = rationale_above(@complexity_check) + + refute rationale == [], "no comment block above the #{@complexity_check} entry" + assert Enum.join(rationale, "\n") =~ ~r/crap/i + end + + # Regression guard: `mix credo.gen.config` would silently drop both of these, + # and the custom check would stop running without one word of warning. + test "the custom check and its load path survive" do + config = default_config() + + assert @custom_check in check_modules(config[:checks][:enabled]) + assert @custom_check_load_path in config[:requires] + end + + defp default_config do + {config, _bindings} = Code.eval_file(@config_path) + Enum.find(config.configs, &(&1.name == "default")) + end + + defp check_modules(checks) do + Enum.map(checks, fn + {module, _opts} -> module_name(module) + module -> module_name(module) + end) + end + + # `Atom.to_string/1` keeps the "Elixir." prefix that `inspect/1` drops; the + # assertions read better (and diff better) without it. + defp module_name(module) do + module |> Atom.to_string() |> String.replace_prefix("Elixir.", "") + end + + # The contiguous `#` block immediately above the check's own line. The entry + # line is the one that is not itself a comment: a comment mentioning the + # check by name must not be mistaken for the entry. + defp rationale_above(check_name) do + lines = @config_path |> File.read!() |> String.split("\n") + + index = + Enum.find_index(lines, fn line -> + String.contains?(line, check_name) and not comment?(line) + end) + + lines + |> Enum.take(index) + |> Enum.reverse() + |> Enum.take_while(&comment?/1) + end + + defp comment?(line), do: line |> String.trim() |> String.starts_with?("#") +end diff --git a/blogex/.credo.exs b/blogex/.credo.exs new file mode 100644 index 0000000..cade726 --- /dev/null +++ b/blogex/.credo.exs @@ -0,0 +1,238 @@ +# This file contains the configuration for Credo and you are probably reading +# this after creating it with `mix credo.gen.config`. +# +# If you find anything wrong or unclear in this file, please report an +# issue on GitHub: https://github.com/rrrene/credo/issues +# +# The gate for this library is `mix credo --strict` on the command line (the +# `credo` target in Makefile). The command line, not this file, decides: +# `--strict` overrides the inert `strict: false` key below. Editing `strict` +# here to silence a finding would change nothing - fix the finding, or write +# down why the check is wrong for this codebase (see `disabled:`). +# +%{ + # + # You can have as many configs as you like in the `configs:` field. + configs: [ + %{ + # + # Run any config using `mix credo -C `. If no config name is given + # "default" is used. + # + name: "default", + # + # These are the files included in the analysis: + files: %{ + # + # You can give explicit globs or simply directories. + # In the latter case `**/*.{ex,exs}` will be used. + # + included: [ + "lib/", + "src/", + "test/", + "web/", + "apps/*/lib/", + "apps/*/src/", + "apps/*/test/", + "apps/*/web/" + ], + excluded: [~r"/_build/", ~r"/deps/", ~r"/node_modules/"] + }, + # + # Load and configure plugins here: + # + plugins: [], + # + # If you create your own checks, you must specify the source files for + # them here, so they can be loaded by Credo before running the analysis. + # + requires: [], + # + # If you want to enforce a style guide and need a more traditional linting + # experience, you can change `strict` to `true` below: + # + strict: false, + # + # To modify the timeout for parsing files, change this value: + # + parse_timeout: 5000, + # + # If you want to use uncolored output by default, you can change `color` + # to `false` below: + # + color: true, + # + # You can customize the parameters of any check by adding a second element + # to the tuple. + # + # To disable a check put `false` as second element: + # + # {Credo.Check.Design.DuplicatedCode, false} + # + checks: %{ + enabled: [ + # + ## Consistency Checks + # + {Credo.Check.Consistency.ExceptionNames, []}, + {Credo.Check.Consistency.LineEndings, []}, + {Credo.Check.Consistency.ParameterPatternMatching, []}, + {Credo.Check.Consistency.SpaceAroundOperators, []}, + {Credo.Check.Consistency.SpaceInParentheses, []}, + {Credo.Check.Consistency.TabsOrSpaces, []}, + + # + ## Design Checks + # + # You can customize the priority of any check + # Priority values are: `low, normal, high, higher` + # + {Credo.Check.Design.AliasUsage, + [priority: :low, if_nested_deeper_than: 2, if_called_more_often_than: 0]}, + {Credo.Check.Design.TagFIXME, []}, + # You can also customize the exit_status of each check. + # If you don't want TODO comments to cause `mix credo` to fail, just + # set this value to 0 (zero). + # + {Credo.Check.Design.TagTODO, [exit_status: 2]}, + + # + ## Readability Checks + # + {Credo.Check.Readability.AliasOrder, []}, + {Credo.Check.Readability.FunctionNames, []}, + {Credo.Check.Readability.LargeNumbers, []}, + {Credo.Check.Readability.MaxLineLength, [priority: :low, max_length: 120]}, + {Credo.Check.Readability.ModuleAttributeNames, []}, + {Credo.Check.Readability.ModuleDoc, []}, + {Credo.Check.Readability.ModuleNames, []}, + {Credo.Check.Readability.ParenthesesInCondition, []}, + {Credo.Check.Readability.ParenthesesOnZeroArityDefs, []}, + {Credo.Check.Readability.PipeIntoAnonymousFunctions, []}, + {Credo.Check.Readability.PredicateFunctionNames, []}, + {Credo.Check.Readability.PreferImplicitTry, []}, + {Credo.Check.Readability.RedundantBlankLines, []}, + {Credo.Check.Readability.Semicolons, []}, + {Credo.Check.Readability.SpaceAfterCommas, []}, + {Credo.Check.Readability.StringSigils, []}, + {Credo.Check.Readability.TrailingBlankLine, []}, + {Credo.Check.Readability.TrailingWhiteSpace, []}, + {Credo.Check.Readability.UnnecessaryAliasExpansion, []}, + {Credo.Check.Readability.VariableNames, []}, + {Credo.Check.Readability.WithSingleClause, []}, + + # + ## Refactoring Opportunities + # + {Credo.Check.Refactor.Apply, []}, + {Credo.Check.Refactor.CondStatements, []}, + {Credo.Check.Refactor.FilterCount, []}, + {Credo.Check.Refactor.FilterFilter, []}, + {Credo.Check.Refactor.FunctionArity, []}, + {Credo.Check.Refactor.LongQuoteBlocks, []}, + {Credo.Check.Refactor.MapJoin, []}, + {Credo.Check.Refactor.MatchInCondition, []}, + {Credo.Check.Refactor.NegatedConditionsInUnless, []}, + {Credo.Check.Refactor.NegatedConditionsWithElse, []}, + {Credo.Check.Refactor.Nesting, []}, + {Credo.Check.Refactor.RedundantWithClauseResult, []}, + {Credo.Check.Refactor.RejectReject, []}, + {Credo.Check.Refactor.UnlessWithElse, []}, + {Credo.Check.Refactor.WithClauses, []}, + + # + ## Warnings + # + {Credo.Check.Warning.ApplicationConfigInModuleAttribute, []}, + {Credo.Check.Warning.BoolOperationOnSameValues, []}, + {Credo.Check.Warning.Dbg, []}, + {Credo.Check.Warning.ExpensiveEmptyEnumCheck, []}, + {Credo.Check.Warning.IExPry, []}, + {Credo.Check.Warning.IoInspect, []}, + {Credo.Check.Warning.MissedMetadataKeyInLoggerConfig, []}, + {Credo.Check.Warning.OperationOnSameValues, []}, + {Credo.Check.Warning.OperationWithConstantResult, []}, + {Credo.Check.Warning.RaiseInsideRescue, []}, + {Credo.Check.Warning.SpecWithStruct, []}, + {Credo.Check.Warning.StructFieldAmount, []}, + {Credo.Check.Warning.UnsafeExec, []}, + {Credo.Check.Warning.UnusedEnumOperation, []}, + {Credo.Check.Warning.UnusedFileOperation, []}, + {Credo.Check.Warning.UnusedKeywordOperation, []}, + {Credo.Check.Warning.UnusedListOperation, []}, + {Credo.Check.Warning.UnusedMapOperation, []}, + {Credo.Check.Warning.UnusedPathOperation, []}, + {Credo.Check.Warning.UnusedRegexOperation, []}, + {Credo.Check.Warning.UnusedStringOperation, []}, + {Credo.Check.Warning.UnusedTupleOperation, []}, + {Credo.Check.Warning.WrongTestFilename, []} + ], + disabled: [ + # + # Our one deliberate deviation from credo's defaults. CRAP (`ex_crap`, + # threshold 30, wired into the Lens pass in Makefile) is this + # library's complexity sensor, and it scores complexity against real + # coverage: a wide `case` that the tests exercise is not a problem - + # that was the itch behind ExCrap. A second, coverage-blind complexity + # sensor would just argue with the first, so complexity questions are + # settled by the CRAP target. Re-enabling this check without retiring + # the CRAP target is a policy change, not a cleanup. + # + {Credo.Check.Refactor.CyclomaticComplexity, []}, + + # + # Checks scheduled for next check update (opt-in for now) + {Credo.Check.Refactor.UtcNowTruncate, []}, + + # + # Controversial and experimental checks (opt-in, just move the check to `:enabled` + # and be sure to use `mix credo --strict` to see low priority checks) + # + {Credo.Check.Consistency.MultiAliasImportRequireUse, []}, + {Credo.Check.Consistency.UnusedVariableNames, []}, + {Credo.Check.Design.DuplicatedCode, []}, + {Credo.Check.Design.SkipTestWithoutComment, []}, + {Credo.Check.Readability.AliasAs, []}, + {Credo.Check.Readability.BlockPipe, []}, + {Credo.Check.Readability.ImplTrue, []}, + {Credo.Check.Readability.MultiAlias, []}, + {Credo.Check.Readability.NestedFunctionCalls, []}, + {Credo.Check.Readability.OneArityFunctionInPipe, []}, + {Credo.Check.Readability.OnePipePerLine, []}, + {Credo.Check.Readability.SeparateAliasRequire, []}, + {Credo.Check.Readability.SingleFunctionToBlockPipe, []}, + {Credo.Check.Readability.SinglePipe, []}, + {Credo.Check.Readability.Specs, []}, + {Credo.Check.Readability.StrictModuleLayout, []}, + {Credo.Check.Readability.WithCustomTaggedTuple, []}, + {Credo.Check.Refactor.ABCSize, []}, + {Credo.Check.Refactor.AppendSingleItem, []}, + {Credo.Check.Refactor.CondInsteadOfIfElse, []}, + {Credo.Check.Refactor.DoubleBooleanNegation, []}, + {Credo.Check.Refactor.FilterReject, []}, + {Credo.Check.Refactor.IoPuts, []}, + {Credo.Check.Refactor.MapMap, []}, + {Credo.Check.Refactor.ModuleDependencies, []}, + {Credo.Check.Refactor.NegatedIsNil, []}, + {Credo.Check.Refactor.PassAsyncInTestCases, []}, + {Credo.Check.Refactor.PipeChainStart, []}, + {Credo.Check.Refactor.RejectFilter, []}, + {Credo.Check.Refactor.VariableRebinding, []}, + {Credo.Check.Warning.LazyLogging, []}, + {Credo.Check.Warning.LeakyEnvironment, []}, + {Credo.Check.Warning.MapGetUnsafePass, []}, + {Credo.Check.Warning.MixEnv, []}, + {Credo.Check.Warning.UnsafeToAtom, []} + # {Credo.Check.Warning.UnusedOperation, [{MyMagicModule, [:fun1, :fun2]}]} + + # {Credo.Check.Refactor.MapInto, []}, + + # + # Custom checks can be created using `mix credo.gen.check`. + # + ] + } + } + ] +} diff --git a/blogex/lib/blogex/link_error.ex b/blogex/lib/blogex/link_error.ex index 8ba9ecb..1282614 100644 --- a/blogex/lib/blogex/link_error.ex +++ b/blogex/lib/blogex/link_error.ex @@ -38,11 +38,9 @@ defmodule Blogex.LinkError do end errors_list = - errors - |> Enum.map(fn {line, link, reason, _meta} -> + Enum.map_join(errors, "\n", fn {line, link, reason, _meta} -> " line #{line}: #{link} — #{reason}" end) - |> Enum.join("\n") """ invalid internal blog links in #{blog}#{post_label} diff --git a/blogex/lib/blogex/link_validator.ex b/blogex/lib/blogex/link_validator.ex index 0a62b00..a51aa56 100644 --- a/blogex/lib/blogex/link_validator.ex +++ b/blogex/lib/blogex/link_validator.ex @@ -129,24 +129,25 @@ defmodule Blogex.LinkValidator do @spec validate_link(String.t()) :: :ok | {:error, String.t()} def validate_link(link) when is_binary(link) do case parse_blog_link(link) do - nil -> - {:error, "not a blog link: #{link}"} - - {blog_id_str, slug_part} -> - case Map.fetch(@valid_blog_ids, blog_id_str) do - {:ok, _blog_atom} -> - if String.starts_with?(slug_part, "tag/") do - validate_tag(String.replace_prefix(slug_part, "tag/", "")) - else - validate_slug(slug_part) - end - - :error -> - {:error, "unknown blog ID: #{blog_id_str}"} - end + nil -> {:error, "not a blog link: #{link}"} + {blog_id_str, path} -> validate_blog_path(blog_id_str, path) end end + # The blog ID must be one this app knows about; only then is the rest of the + # path worth looking at. Extracted so `validate_link/1` stays one `case` deep: + # the branching here is data (which part of the path is this?), not control. + @spec validate_blog_path(String.t(), String.t()) :: :ok | {:error, String.t()} + defp validate_blog_path(blog_id_str, path) do + case Map.fetch(@valid_blog_ids, blog_id_str) do + {:ok, _blog_atom} -> validate_path(path) + :error -> {:error, "unknown blog ID: #{blog_id_str}"} + end + end + + defp validate_path("tag/" <> tag), do: validate_tag(tag) + defp validate_path(slug), do: validate_slug(slug) + @doc """ Validates a list of links, returning errors with line numbers. @@ -221,11 +222,13 @@ defmodule Blogex.LinkValidator do @doc false @spec parse_blog_link(String.t()) :: {String.t(), String.t()} | nil def parse_blog_link(path) do - with ["", "blog", blog_id, rest] <- String.split(path, "/", parts: 4) do - slug = String.slice(rest, 0, slug_end(rest)) - {blog_id, slug} - else - _ -> nil + case String.split(path, "/", parts: 4) do + ["", "blog", blog_id, rest] -> + slug = String.slice(rest, 0, slug_end(rest)) + {blog_id, slug} + + _ -> + nil end end diff --git a/blogex/lib/blogex/router.ex b/blogex/lib/blogex/router.ex index 383b0d9..b7249c6 100644 --- a/blogex/lib/blogex/router.ex +++ b/blogex/lib/blogex/router.ex @@ -34,12 +34,15 @@ defmodule Blogex.Router do use Plug.Router + alias Blogex.Router.Helpers + alias Phoenix.HTML.Safe + plug :match plug :dispatch get "/feed.xml" do blog = conn.private[:blogex_blog] - base_url = Blogex.Router.Helpers.base_url(conn) + base_url = Helpers.base_url(conn) xml = Blogex.Feed.rss(blog, base_url) @@ -50,7 +53,7 @@ defmodule Blogex.Router do get "/atom.xml" do blog = conn.private[:blogex_blog] - base_url = Blogex.Router.Helpers.base_url(conn) + base_url = Helpers.base_url(conn) xml = Blogex.Feed.atom(blog, base_url) @@ -164,7 +167,7 @@ defmodule Blogex.Router do defp send_html(conn, rendered) do html = rendered - |> Phoenix.HTML.Safe.to_iodata() + |> Safe.to_iodata() |> IO.iodata_to_binary() conn diff --git a/blogex/mix.exs b/blogex/mix.exs index 4a0a8bf..08097cd 100644 --- a/blogex/mix.exs +++ b/blogex/mix.exs @@ -52,6 +52,7 @@ defmodule Blogex.MixProject do {:boundary, "~> 0.11", runtime: false}, {:ex_doc, "~> 0.34", only: :dev, runtime: false}, {:rustler, "~> 0.38.0", optional: true}, + {:credo, "~> 1.7", only: [:dev, :test], runtime: false}, {:ex_crap, "~> 0.1.1", only: [:dev, :test], runtime: false}, {:muex, "~> 0.12", only: [:dev, :test], runtime: false} ] diff --git a/blogex/mix.lock b/blogex/mix.lock index 3a6d5f5..8ec8199 100644 --- a/blogex/mix.lock +++ b/blogex/mix.lock @@ -1,9 +1,12 @@ %{ "boundary": {:hex, :boundary, "0.11.0", "56cff565b5565930765e02782281b022935fe8c2e1504543c8c1b44b85893b40", [:mix], [], "hexpm", "b9297a96ea14c0e63dbd77b8135e0d483f9c6f34ed1c9dee4b3fea82c596f55a"}, + "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"}, + "credo": {:hex, :credo, "1.7.19", "cc52129665fc7c15143d47838fda0f9cd6dac9ceced7bf4da6f85fcbfe64b12a", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "2d8bc95d5a7bb99dd2613621d4f08c6a3575c3fd4b62e6a2b48a100352a557b8"}, "earmark": {:hex, :earmark, "1.4.48", "5f41e579d85ef812351211842b6e005f6e0cef111216dea7d4b9d58af4608434", [:mix], [], "hexpm", "a461a0ddfdc5432381c876af1c86c411fd78a25790c75023c7a4c035fdc858f9"}, "earmark_parser": {:hex, :earmark_parser, "1.4.44", "f20830dd6b5c77afe2b063777ddbbff09f9759396500cdbe7523efd58d7a339c", [:mix], [], "hexpm", "4778ac752b4701a5599215f7030989c989ffdc4f6df457c5f36938cc2d2a2750"}, "ex_crap": {:hex, :ex_crap, "0.1.1", "c1770e7ce1843f304a27f5d32bfa867a340140e337684ee1929c770b2499f884", [:mix], [{:boundary, "~> 0.10", [hex: :boundary, repo: "hexpm", optional: false]}], "hexpm", "ada5d59bce442c1a8a987cb5328cbf4469fb56eb8d04641886f2199569a75b12"}, "ex_doc": {:hex, :ex_doc, "0.40.1", "67542e4b6dde74811cfd580e2c0149b78010fd13001fda7cfeb2b2c2ffb1344d", [:mix], [{:earmark_parser, "~> 1.4.44", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "bcef0e2d360d93ac19f01a85d58f91752d930c0a30e2681145feea6bd3516e00"}, + "file_system": {:hex, :file_system, "1.1.1", "31864f4685b0148f25bd3fbef2b1228457c0c89024ad67f7a81a3ffbc0bbad3a", [:mix], [], "hexpm", "7a15ff97dfe526aeefb090a7a9d3d03aa907e100e262a0f8f7746b78f8f87a5d"}, "floki": {:hex, :floki, "0.38.4", "10f98971e892aed2c2f1b3a0f928e488e3797e1c6dd3dfd98db40b14e9a78bcf", [:mix], [], "hexpm", "bdb34645eee8e79845c7edaca2d4099a52804ee4d4a3ecc683a69451f0244973"}, "jason": {:hex, :jason, "1.4.5", "2e3a008590b0b8d7388c20293e9dcc9cf3e5d642fd2a114e4cbbb52e595d940a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0 or ~> 3.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "b0c823996102bcd0239b3c2444eb00409b72f6a140c1950bc8b457d836b30684"}, "makeup": {:hex, :makeup, "1.2.2", "882d46dc0905e9ff7abf2aab61a7e6b3dcc555533977d8a23b06019e6c89ac94", [:mix], [{:nimble_parsec, "~> 1.4", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "9a1a24e5b343b8ae16abea0822c10a6f75da27af7fa802ada5251f7579bfccfa"}, diff --git a/blogex/test/blogex/link_error_test.exs b/blogex/test/blogex/link_error_test.exs new file mode 100644 index 0000000..8bc9aff --- /dev/null +++ b/blogex/test/blogex/link_error_test.exs @@ -0,0 +1,44 @@ +defmodule Blogex.LinkErrorTest do + @moduledoc """ + Pins the rendered exception message, not just the struct. + + The message is what a compile-time failure shows a human: the indentation and + the newline between errors are load-bearing. This is a characterization test + written before `message/1` was rewritten (`Enum.map/2 |> Enum.join/2` -> + `Enum.map_join/3`), so it passes on both sides of that change by design - it + exists to make the rewrite safe, not to prove new behaviour. + """ + use ExUnit.Case, async: true + + test "message/1 renders one indented line per error" do + error = %Blogex.LinkError{ + blog: :engineering, + post_id: "my-post", + errors: [ + {1, "/blog/unknown/broken", "unknown blog ID: unknown", [post_id: "my-post"]}, + {7, "/blog/engineering/", "empty slug", [post_id: "my-post"]} + ] + } + + assert Exception.message(error) == """ + invalid internal blog links in engineering (post: my-post) + + line 1: /blog/unknown/broken — unknown blog ID: unknown + line 7: /blog/engineering/ — empty slug + """ + end + + test "message/1 omits the post label when there is no post id" do + error = %Blogex.LinkError{ + blog: :engineering, + post_id: nil, + errors: [{3, "/blog/unknown/broken", "unknown blog ID: unknown", []}] + } + + assert Exception.message(error) == """ + invalid internal blog links in engineering + + line 3: /blog/unknown/broken — unknown blog ID: unknown + """ + end +end diff --git a/blogex/test/blogex/link_validator_test.exs b/blogex/test/blogex/link_validator_test.exs index 1fc8198..5307d9f 100644 --- a/blogex/test/blogex/link_validator_test.exs +++ b/blogex/test/blogex/link_validator_test.exs @@ -355,5 +355,12 @@ defmodule Blogex.LinkValidatorTest do assert LinkValidator.parse_blog_link("/blog/engineering/hello-world#top") == {"engineering", "hello-world"} end + + # Only the paths that cannot be a blog link return nil; a nested slug + # ("/blog/eng/a/post") still parses and is rejected later by validate_slug/1. + test "returns nil for anything that is not /blog//" do + refute LinkValidator.parse_blog_link("/blog/engineering") + refute LinkValidator.parse_blog_link("/notblog/engineering/a-post") + end end end diff --git a/blogex/test/credo_config_test.exs b/blogex/test/credo_config_test.exs new file mode 100644 index 0000000..fd987e5 --- /dev/null +++ b/blogex/test/credo_config_test.exs @@ -0,0 +1,66 @@ +defmodule Blogex.CredoConfigTest do + @moduledoc """ + Pins the *policy* in `.credo.exs`, not just the file's presence. + + The gate is `mix credo --strict` (see the `credo` target in `Makefile`), so + the config is load-bearing: a check silently re-enabled here changes what + every future commit has to satisfy. These assertions are deliberately coarse + - "this check sits in that list", "the disable says why" - so they survive + credo's own boilerplate churn but not a policy change. + """ + use ExUnit.Case, async: true + + @config_path ".credo.exs" + @complexity_check "Credo.Check.Refactor.CyclomaticComplexity" + + test "cyclomatic complexity is disabled in favour of CRAP" do + checks = default_config()[:checks] + + assert @complexity_check in check_modules(checks[:disabled]) + refute @complexity_check in check_modules(checks[:enabled]) + end + + test "the disable carries a rationale comment" do + rationale = rationale_above(@complexity_check) + + refute rationale == [], "no comment block above the #{@complexity_check} entry" + assert Enum.join(rationale, "\n") =~ ~r/crap/i + end + + defp default_config do + {config, _bindings} = Code.eval_file(@config_path) + Enum.find(config.configs, &(&1.name == "default")) + end + + defp check_modules(checks) do + Enum.map(checks, fn + {module, _opts} -> module_name(module) + module -> module_name(module) + end) + end + + # `Atom.to_string/1` keeps the "Elixir." prefix that `inspect/1` drops; the + # assertions read better (and diff better) without it. + defp module_name(module) do + module |> Atom.to_string() |> String.replace_prefix("Elixir.", "") + end + + # The contiguous `#` block immediately above the check's own line. The entry + # line is the one that is not itself a comment: a comment mentioning the + # check by name must not be mistaken for the entry. + defp rationale_above(check_name) do + lines = @config_path |> File.read!() |> String.split("\n") + + index = + Enum.find_index(lines, fn line -> + String.contains?(line, check_name) and not comment?(line) + end) + + lines + |> Enum.take(index) + |> Enum.reverse() + |> Enum.take_while(&comment?/1) + end + + defp comment?(line), do: line |> String.trim() |> String.starts_with?("#") +end diff --git a/blogex/test/support/blogex_test_blogs.ex b/blogex/test/support/blogex_test_blogs.ex index f02aae5..8c68211 100644 --- a/blogex/test/support/blogex_test_blogs.ex +++ b/blogex/test/support/blogex_test_blogs.ex @@ -10,17 +10,19 @@ defmodule Blogex.Test.AlphaBlog do tests can assert exact orderings rather than re-deriving them. """ + alias Blogex.Test.PostBuilder + def blog_id, do: :alpha def all_posts, - do: [Blogex.Test.PostBuilder.build(id: "a1", date: ~D[2026-03-01], blog: :alpha)] + do: [PostBuilder.build(id: "a1", date: ~D[2026-03-01], blog: :alpha)] def all_tags, do: ["elixir"] def unfiltered_posts, do: [ - Blogex.Test.PostBuilder.build(id: "a1", date: ~D[2026-03-01], blog: :alpha), - Blogex.Test.PostBuilder.build( + PostBuilder.build(id: "a1", date: ~D[2026-03-01], blog: :alpha), + PostBuilder.build( id: "a-draft", date: ~D[2026-03-05], blog: :alpha, @@ -34,16 +36,18 @@ defmodule Blogex.Test.BetaBlog do The second blog module for cross-blog tests; see `Blogex.Test.AlphaBlog`. """ + alias Blogex.Test.PostBuilder + def blog_id, do: :beta def all_posts, - do: [Blogex.Test.PostBuilder.build(id: "b1", date: ~D[2026-03-15], blog: :beta)] + do: [PostBuilder.build(id: "b1", date: ~D[2026-03-15], blog: :beta)] def all_tags, do: ["devops"] def unfiltered_posts, do: [ - Blogex.Test.PostBuilder.build(id: "b1", date: ~D[2026-03-15], blog: :beta), - Blogex.Test.PostBuilder.build(id: "b-future", date: ~D[2099-01-01], blog: :beta) + PostBuilder.build(id: "b1", date: ~D[2026-03-15], blog: :beta), + PostBuilder.build(id: "b-future", date: ~D[2099-01-01], blog: :beta) ] end diff --git a/blogex/test/support/setup.ex b/blogex/test/support/setup.ex index 7dc15fa..1866bcd 100644 --- a/blogex/test/support/setup.ex +++ b/blogex/test/support/setup.ex @@ -5,6 +5,8 @@ defmodule Blogex.Test.Setup do import Blogex.Test.PostBuilder + alias Blogex.Test.FakeBlog + @doc """ Starts a FakeBlog with a standard set of posts. Returns `%{blog: module, posts: posts}`. @@ -13,9 +15,9 @@ defmodule Blogex.Test.Setup do posts = Keyword.get(opts, :posts, default_posts()) blog_opts = Keyword.drop(opts, [:posts]) - {:ok, _} = Blogex.Test.FakeBlog.start(posts, blog_opts) + {:ok, _} = FakeBlog.start(posts, blog_opts) - Map.merge(context, %{blog: Blogex.Test.FakeBlog, posts: posts}) + Map.merge(context, %{blog: FakeBlog, posts: posts}) end @doc "A small set of posts covering common scenarios."