From 0acfb9ac92f4121b41f0ad126fce0833e6a6464b Mon Sep 17 00:00:00 2001 From: pnezis Date: Mon, 21 Oct 2024 14:04:18 +0300 Subject: [PATCH] Refactor workspace filtering --- workspace/lib/workspace/filtering.ex | 47 +++++-- workspace/lib/workspace/project.ex | 6 + workspace/mix.exs | 2 +- workspace/test/workspace/filtering_test.exs | 134 ++++++++++---------- 4 files changed, 108 insertions(+), 81 deletions(-) diff --git a/workspace/lib/workspace/filtering.ex b/workspace/lib/workspace/filtering.ex index e63f68b..b0b5828 100644 --- a/workspace/lib/workspace/filtering.ex +++ b/workspace/lib/workspace/filtering.ex @@ -85,9 +85,7 @@ defmodule Workspace.Filtering do paths: opts[:paths] ] - Enum.map(workspace.projects, fn {_name, project} -> - Map.put(project, :skip, skippable?(workspace, project, opts)) - end) + Enum.map(workspace.projects, fn {_name, project} -> maybe_skip(workspace, project, opts) end) end defp maybe_to_atom(nil), do: nil @@ -115,17 +113,40 @@ defmodule Workspace.Filtering do do: raise(ArgumentError, "invalid tag, it should be a string of the form `tag` or `scope:tag`") - defp skippable?(workspace, project, opts) do - excluded_project?(project, opts[:excluded]) or - excluded_tag?(project, opts[:excluded_tags]) or - not_selected_project?(project, opts[:selected]) or - not_selected_tag?(project, opts[:tags]) or - not_in_paths?(project, opts[:paths]) or - not_root?(project, opts[:only_roots]) or - not_affected?(project, opts[:affected]) or - not_modified?(project, opts[:modified]) or - not_dependency?(workspace, project, opts[:dependency]) or + defp maybe_skip(workspace, project, opts) do + project + |> skip_if(workspace, fn project, _workspace -> + excluded_project?(project, opts[:excluded]) + end) + |> skip_if(workspace, fn project, _workspace -> + excluded_tag?(project, opts[:excluded_tags]) + end) + |> skip_if(workspace, fn project, _workspace -> + not_selected_project?(project, opts[:selected]) + end) + |> skip_if(workspace, fn project, _workspace -> not_selected_tag?(project, opts[:tags]) end) + |> skip_if(workspace, fn project, _workspace -> not_in_paths?(project, opts[:paths]) end) + |> skip_if(workspace, fn project, _workspace -> not_root?(project, opts[:only_roots]) end) + |> skip_if(workspace, fn project, _workspace -> not_affected?(project, opts[:affected]) end) + |> skip_if(workspace, fn project, _workspace -> not_modified?(project, opts[:modified]) end) + |> skip_if(workspace, fn project, workspace -> + not_dependency?(workspace, project, opts[:dependency]) + end) + |> skip_if(workspace, fn project, workspace -> not_dependent?(workspace, project, opts[:dependent]) + end) + end + + # if the project is already skipped there is no reason to check again + # otherwise we evaluate the condition and skip accordingly + defp skip_if(%Workspace.Project{skip: true} = project, _workspace, _skip_condition), do: project + + defp skip_if(project, workspace, skip_condition) do + if skip_condition.(project, workspace) do + Workspace.Project.skip(project) + else + project + end end defp excluded_project?(project, excluded), do: project.app in excluded diff --git a/workspace/lib/workspace/project.ex b/workspace/lib/workspace/project.ex index b7ae400..86fb6c6 100644 --- a/workspace/lib/workspace/project.ex +++ b/workspace/lib/workspace/project.ex @@ -215,6 +215,12 @@ defmodule Workspace.Project do def set_root(project, root?) when is_boolean(root?), do: %Workspace.Project{project | root?: root?} + @doc """ + Marks the given project as skippable. + """ + @spec skip(project :: t()) :: t() + def skip(project), do: %Workspace.Project{project | skip: true} + @doc """ Marks the given project as `:modified`. diff --git a/workspace/mix.exs b/workspace/mix.exs index 064d2d9..50da097 100644 --- a/workspace/mix.exs +++ b/workspace/mix.exs @@ -18,7 +18,7 @@ defmodule Workspace.MixProject do # Tests test_coverage: [ - threshold: 98, + threshold: 97, export: "workspace", output: "../artifacts/coverdata/workspace" ], diff --git a/workspace/test/workspace/filtering_test.exs b/workspace/test/workspace/filtering_test.exs index f184378..1e337f3 100644 --- a/workspace/test/workspace/filtering_test.exs +++ b/workspace/test/workspace/filtering_test.exs @@ -16,94 +16,94 @@ defmodule Workspace.FilteringTest do describe "run/2" do test "filters and updates the given workspace", %{workspace: workspace} do - workspace = Workspace.Filtering.run(workspace, exclude: [:bar]) + filtered = Workspace.Filtering.run(workspace, exclude: [:bar]) - assert workspace.projects[:bar].skip - refute workspace.projects[:foo].skip - refute workspace.projects[:baz].skip + assert filtered.projects[:bar].skip + refute filtered.projects[:foo].skip + refute filtered.projects[:baz].skip end test "if app is excluded skips the project", %{workspace: workspace} do - workspace = Workspace.Filtering.run(workspace, exclude: ["bar"]) + filtered = Workspace.Filtering.run(workspace, exclude: ["bar"]) - assert Workspace.project!(workspace, :bar).skip - refute Workspace.project!(workspace, :foo).skip - refute Workspace.project!(workspace, :baz).skip + assert Workspace.project!(filtered, :bar).skip + refute Workspace.project!(filtered, :foo).skip + refute Workspace.project!(filtered, :baz).skip end test "if app in selected it is not skipped - everything else is skipped", %{ workspace: workspace } do - workspace = Workspace.Filtering.run(workspace, project: ["bar"]) + filtered = Workspace.Filtering.run(workspace, project: ["bar"]) - refute Workspace.project!(workspace, :bar).skip - assert Workspace.project!(workspace, :foo).skip - assert Workspace.project!(workspace, :baz).skip + refute Workspace.project!(filtered, :bar).skip + assert Workspace.project!(filtered, :foo).skip + assert Workspace.project!(filtered, :baz).skip end test "ignore has priority over project", %{ workspace: workspace } do - workspace = Workspace.Filtering.run(workspace, exclude: [:bar], project: [:bar]) + filtered = Workspace.Filtering.run(workspace, exclude: [:bar], project: [:bar]) - assert Workspace.project!(workspace, :bar).skip - assert Workspace.project!(workspace, :foo).skip + assert Workspace.project!(filtered, :bar).skip + assert Workspace.project!(filtered, :foo).skip end test "with a single excluded tag", %{ workspace: workspace } do - workspace = Workspace.Filtering.run(workspace, excluded_tags: [:bar]) + filtered = Workspace.Filtering.run(workspace, excluded_tags: [:bar]) - assert Workspace.project!(workspace, :bar).skip - assert Workspace.project!(workspace, :baz).skip - refute Workspace.project!(workspace, :foo).skip + assert Workspace.project!(filtered, :bar).skip + assert Workspace.project!(filtered, :baz).skip + refute Workspace.project!(filtered, :foo).skip end test "with multiple excluded tags", %{ workspace: workspace } do - workspace = Workspace.Filtering.run(workspace, excluded_tags: [:bar, :foo]) + filtered = Workspace.Filtering.run(workspace, excluded_tags: [:bar, :foo]) - assert Workspace.project!(workspace, :bar).skip - assert Workspace.project!(workspace, :baz).skip - assert Workspace.project!(workspace, :foo).skip + assert Workspace.project!(filtered, :bar).skip + assert Workspace.project!(filtered, :baz).skip + assert Workspace.project!(filtered, :foo).skip end test "with a single selected tag", %{ workspace: workspace } do - workspace = Workspace.Filtering.run(workspace, tags: [:bar]) + filtered = Workspace.Filtering.run(workspace, tags: [:bar]) - refute Workspace.project!(workspace, :bar).skip - refute Workspace.project!(workspace, :baz).skip - assert Workspace.project!(workspace, :foo).skip + refute Workspace.project!(filtered, :bar).skip + refute Workspace.project!(filtered, :baz).skip + assert Workspace.project!(filtered, :foo).skip end test "with multiple selected tags", %{ workspace: workspace } do - workspace = Workspace.Filtering.run(workspace, tags: [:bar, :foo]) + filtered = Workspace.Filtering.run(workspace, tags: [:bar, :foo]) - refute Workspace.project!(workspace, :bar).skip - refute Workspace.project!(workspace, :baz).skip - refute Workspace.project!(workspace, :foo).skip + refute Workspace.project!(filtered, :bar).skip + refute Workspace.project!(filtered, :baz).skip + refute Workspace.project!(filtered, :foo).skip end test "with scoped tags", %{workspace: workspace} do - workspace = Workspace.Filtering.run(workspace, tags: [{:scope, :ui}]) + filtered = Workspace.Filtering.run(workspace, tags: [{:scope, :ui}]) - assert Workspace.project!(workspace, :bar).skip - assert Workspace.project!(workspace, :baz).skip - refute Workspace.project!(workspace, :foo).skip + assert Workspace.project!(filtered, :bar).skip + assert Workspace.project!(filtered, :baz).skip + refute Workspace.project!(filtered, :foo).skip end test "with binary tags", %{workspace: workspace} do - workspace = Workspace.Filtering.run(workspace, tags: ["scope:ui", "bar"]) + filtered = Workspace.Filtering.run(workspace, tags: ["scope:ui", "bar"]) - refute Workspace.project!(workspace, :bar).skip - refute Workspace.project!(workspace, :baz).skip - refute Workspace.project!(workspace, :foo).skip + refute Workspace.project!(filtered, :bar).skip + refute Workspace.project!(filtered, :baz).skip + refute Workspace.project!(filtered, :foo).skip end test "with invalid tags", %{workspace: workspace} do @@ -119,51 +119,51 @@ defmodule Workspace.FilteringTest do end test "with paths set", %{workspace: workspace} do - workspace = Workspace.Filtering.run(workspace, paths: ["packages"]) + filtered = Workspace.Filtering.run(workspace, paths: ["packages"]) - refute Workspace.project!(workspace, :bar).skip - refute Workspace.project!(workspace, :baz).skip - refute Workspace.project!(workspace, :foo).skip + refute Workspace.project!(filtered, :bar).skip + refute Workspace.project!(filtered, :baz).skip + refute Workspace.project!(filtered, :foo).skip - workspace = Workspace.Filtering.run(workspace, paths: ["packages/foo", "packages/baz"]) + filtered = Workspace.Filtering.run(workspace, paths: ["packages/foo", "packages/baz"]) - assert Workspace.project!(workspace, :bar).skip - refute Workspace.project!(workspace, :baz).skip - refute Workspace.project!(workspace, :foo).skip + assert Workspace.project!(filtered, :bar).skip + refute Workspace.project!(filtered, :baz).skip + refute Workspace.project!(filtered, :foo).skip - workspace = Workspace.Filtering.run(workspace, paths: ["packages/food", "packages/baze"]) + filtered = Workspace.Filtering.run(workspace, paths: ["packages/food", "packages/baze"]) - assert Workspace.project!(workspace, :bar).skip - assert Workspace.project!(workspace, :baz).skip - assert Workspace.project!(workspace, :foo).skip + assert Workspace.project!(filtered, :bar).skip + assert Workspace.project!(filtered, :baz).skip + assert Workspace.project!(filtered, :foo).skip end test "with dependency set", %{workspace: workspace} do - workspace = Workspace.Filtering.run(workspace, dependency: :invalid) + filtered = Workspace.Filtering.run(workspace, dependency: :invalid) - assert Workspace.project!(workspace, :bar).skip - assert Workspace.project!(workspace, :baz).skip - assert Workspace.project!(workspace, :foo).skip + assert Workspace.project!(filtered, :bar).skip + assert Workspace.project!(filtered, :baz).skip + assert Workspace.project!(filtered, :foo).skip - workspace = Workspace.Filtering.run(workspace, dependency: :bar) + filtered = Workspace.Filtering.run(workspace, dependency: :bar) - assert Workspace.project!(workspace, :bar).skip - assert Workspace.project!(workspace, :baz).skip - refute Workspace.project!(workspace, :foo).skip + assert Workspace.project!(filtered, :bar).skip + assert Workspace.project!(filtered, :baz).skip + refute Workspace.project!(filtered, :foo).skip end test "with dependent set", %{workspace: workspace} do - workspace = Workspace.Filtering.run(workspace, dependent: :bar) + filtered = Workspace.Filtering.run(workspace, dependent: :bar) - assert Workspace.project!(workspace, :bar).skip - assert Workspace.project!(workspace, :baz).skip - assert Workspace.project!(workspace, :foo).skip + assert Workspace.project!(filtered, :bar).skip + assert Workspace.project!(filtered, :baz).skip + assert Workspace.project!(filtered, :foo).skip - workspace = Workspace.Filtering.run(workspace, dependent: :foo) + filtered = Workspace.Filtering.run(workspace, dependent: :foo) - refute Workspace.project!(workspace, :bar).skip - assert Workspace.project!(workspace, :baz).skip - assert Workspace.project!(workspace, :foo).skip + refute Workspace.project!(filtered, :bar).skip + assert Workspace.project!(filtered, :baz).skip + assert Workspace.project!(filtered, :foo).skip end end end