From 0759e3098eac9edd249e29058fb78dedc9eb611e Mon Sep 17 00:00:00 2001 From: devdanzin <74280297+devdanzin@users.noreply.github.com> Date: Tue, 11 Jul 2023 21:20:31 -0300 Subject: [PATCH 1/6] Allow passing multiple file names to graph. --- src/wily/__main__.py | 4 ++-- src/wily/commands/graph.py | 19 +++++++++++-------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/wily/__main__.py b/src/wily/__main__.py index 1b4c7ef0..0fd92ddd 100644 --- a/src/wily/__main__.py +++ b/src/wily/__main__.py @@ -368,8 +368,8 @@ def diff(ctx, files, metrics, all, detail, revision): """ ) ) -@click.argument("path", type=click.Path(resolve_path=False)) -@click.argument("metrics", nargs=-1, required=True) +@click.argument("path", nargs=-1, type=click.Path(resolve_path=False)) +@click.argument("metrics", nargs=1, required=True) @click.option( "-o", "--output", diff --git a/src/wily/commands/graph.py b/src/wily/commands/graph.py index c4ec1abe..12763c84 100644 --- a/src/wily/commands/graph.py +++ b/src/wily/commands/graph.py @@ -3,6 +3,7 @@ TODO: Add multiple lines for multiple files """ +import pathlib import plotly.graph_objs as go import plotly.offline @@ -35,10 +36,10 @@ def graph( :type config: :class:`wily.config.WilyConfig` :param path: The path to the files. - :type path: ``list`` + :type path: ``tuple`` :param metrics: The Y and Z-axis metrics to report on. - :type metrics: ``tuple`` + :type metrics: ``str`` :param output: Save report to specified path instead of opening browser. :type output: ``str`` @@ -53,21 +54,22 @@ def graph( else: x_operator, x_key = metric_parts(x_axis) + metrics = metrics.split(",") + y_metric = resolve_metric(metrics[0]) - title = f"{x_axis.capitalize()} of {y_metric.description} for {path}{' aggregated' if aggregate else ''}" + title = f"{x_axis.capitalize()} of {y_metric.description} for {', '.join(path)}{' aggregated' if aggregate else ''}" if not aggregate: tracked_files = set() for rev in state.index[state.default_archiver].revisions: tracked_files.update(rev.revision.tracked_files) - paths = { + paths = tuple( tracked_file for tracked_file in tracked_files - if tracked_file.startswith(path) - } or {path} + if any(tracked_file.startswith(p) for p in path) + ) or path else: - paths = {path} - + paths = path operator, key = metric_parts(metrics[0]) if len(metrics) == 1: # only y-axis z_axis = None @@ -75,6 +77,7 @@ def graph( z_axis = resolve_metric(metrics[1]) z_operator, z_key = metric_parts(metrics[1]) for path in paths: + path = pathlib.Path(path) x = [] y = [] z = [] From a3ab26cf1714eb582a947fb4db15a0e2b00f2117 Mon Sep 17 00:00:00 2001 From: devdanzin <74280297+devdanzin@users.noreply.github.com> Date: Tue, 15 Aug 2023 17:39:15 -0300 Subject: [PATCH 2/6] Make 'metrics' a required option to CLI graph(), update tests. --- src/wily/__main__.py | 13 ++++++--- test/integration/test_graph.py | 52 +++++++++++++++++++++++++--------- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/src/wily/__main__.py b/src/wily/__main__.py index 92d9f427..8f3bd356 100644 --- a/src/wily/__main__.py +++ b/src/wily/__main__.py @@ -386,20 +386,25 @@ def diff(ctx, files, metrics, all, detail, revision, wrap): Graph all .py files within src/ for the raw.loc metric - $ wily graph src/ raw.loc + $ wily graph src/ -m raw.loc Graph test.py against raw.loc and cyclomatic.complexity metrics - $ wily graph src/test.py raw.loc cyclomatic.complexity + $ wily graph src/test.py -m raw.loc,cyclomatic.complexity Graph test.py against raw.loc and raw.sloc on the x-axis - $ wily graph src/test.py raw.loc --x-axis raw.sloc + $ wily graph src/test.py -m raw.loc --x-axis raw.sloc """ ) ) @click.argument("path", nargs=-1, type=click.Path(resolve_path=False)) -@click.argument("metrics", nargs=1, required=True) +@click.option( + "-m", + "--metrics", + required=True, + help=_("Comma-separated list of metrics, see list-metrics for choices"), +) @click.option( "-o", "--output", diff --git a/test/integration/test_graph.py b/test/integration/test_graph.py index d6743afa..7596f1b6 100644 --- a/test/integration/test_graph.py +++ b/test/integration/test_graph.py @@ -22,17 +22,25 @@ def test_graph_no_cache(tmpdir, cache_path): with patch.dict("os.environ", values=PATCHED_ENV, clear=True): result = runner.invoke( main.cli, - ["--path", tmpdir, "--cache", cache_path, "graph", _path, "raw.loc"], + ["--path", tmpdir, "--cache", cache_path, "graph", _path, "-m", "raw.loc"], ) assert result.exit_code == 1, result.stdout +def test_graph_no_path(builddir): + """Test the graph feature with no path given""" + runner = CliRunner() + with patch.dict("os.environ", values=PATCHED_ENV, clear=True): + result = runner.invoke(main.cli, ["--path", builddir, "graph", "-m", "raw.loc"]) + assert result.exit_code == 1, result.stdout + + def test_graph(builddir): """Test the graph feature""" runner = CliRunner() with patch.dict("os.environ", values=PATCHED_ENV, clear=True): result = runner.invoke( - main.cli, ["--path", builddir, "graph", _path, "raw.loc"] + main.cli, ["--path", builddir, "graph", _path, "-m", "raw.loc"] ) assert result.exit_code == 0, result.stdout @@ -42,7 +50,7 @@ def test_graph_all(builddir): runner = CliRunner() with patch.dict("os.environ", values=PATCHED_ENV, clear=True): result = runner.invoke( - main.cli, ["--path", builddir, "graph", _path, "raw.loc", "--all"] + main.cli, ["--path", builddir, "graph", _path, "-m", "raw.loc", "--all"] ) assert result.exit_code == 0, result.stdout @@ -52,7 +60,7 @@ def test_graph_all_with_shorthand_metric(builddir): runner = CliRunner() with patch.dict("os.environ", values=PATCHED_ENV, clear=True): result = runner.invoke( - main.cli, ["--path", builddir, "graph", _path, "loc", "--all"] + main.cli, ["--path", builddir, "graph", _path, "-m", "loc", "--all"] ) assert result.exit_code == 0, result.stdout @@ -62,7 +70,7 @@ def test_graph_changes(builddir): runner = CliRunner() with patch.dict("os.environ", values=PATCHED_ENV, clear=True): result = runner.invoke( - main.cli, ["--path", builddir, "graph", _path, "raw.loc", "--changes"] + main.cli, ["--path", builddir, "graph", _path, "-m", "raw.loc", "--changes"] ) assert result.exit_code == 0, result.stdout @@ -72,7 +80,8 @@ def test_graph_custom_x(builddir): runner = CliRunner() with patch.dict("os.environ", values=PATCHED_ENV, clear=True): result = runner.invoke( - main.cli, ["--path", builddir, "graph", _path, "raw.loc", "-x", "raw.sloc"] + main.cli, + ["--path", builddir, "graph", _path, "-m", "raw.loc", "-x", "raw.sloc"], ) assert result.exit_code == 0, result.stdout @@ -82,7 +91,8 @@ def test_graph_aggregate(builddir): runner = CliRunner() with patch.dict("os.environ", values=PATCHED_ENV, clear=True): result = runner.invoke( - main.cli, ["--path", builddir, "graph", _path, "raw.loc", "--aggregate"] + main.cli, + ["--path", builddir, "graph", _path, "-m", "raw.loc", "--aggregate"], ) assert result.exit_code == 0, result.stdout @@ -92,7 +102,8 @@ def test_graph_individual(builddir): runner = CliRunner() with patch.dict("os.environ", values=PATCHED_ENV, clear=True): result = runner.invoke( - main.cli, ["--path", builddir, "graph", _path, "raw.loc", "--individual"] + main.cli, + ["--path", builddir, "graph", _path, "-m", "raw.loc", "--individual"], ) assert result.exit_code == 0, result.stdout @@ -102,7 +113,7 @@ def test_graph_path(builddir): runner = CliRunner() with patch.dict("os.environ", values=PATCHED_ENV, clear=True): result = runner.invoke( - main.cli, ["--path", builddir, "graph", "src/", "raw.loc"] + main.cli, ["--path", builddir, "graph", "src/", "-m", "raw.loc"] ) assert result.exit_code == 0, result.stdout @@ -112,7 +123,8 @@ def test_graph_multiple(builddir): runner = CliRunner() with patch.dict("os.environ", values=PATCHED_ENV, clear=True): result = runner.invoke( - main.cli, ["--path", builddir, "graph", _path, "raw.loc", "raw.comments"] + main.cli, + ["--path", builddir, "graph", _path, "-m", "raw.loc,raw.comments"], ) assert result.exit_code == 0, result.stdout @@ -128,8 +140,8 @@ def test_graph_multiple_custom_x(builddir): builddir, "graph", _path, - "raw.loc", - "raw.comments", + "-m", + "raw.loc,raw.comments", "-x", "raw.sloc", ], @@ -142,7 +154,8 @@ def test_graph_multiple_path(builddir): runner = CliRunner() with patch.dict("os.environ", values=PATCHED_ENV, clear=True): result = runner.invoke( - main.cli, ["--path", builddir, "graph", "src/", "raw.loc", "raw.comments"] + main.cli, + ["--path", builddir, "graph", "src/", "-m", "raw.loc,raw.comments"], ) assert result.exit_code == 0, result.stdout @@ -159,6 +172,7 @@ def test_graph_output(builddir): builddir, "graph", _path, + "-m", "raw.loc", "-o", "test.html", @@ -180,9 +194,21 @@ def test_graph_output_granular(builddir): builddir, "graph", "src/test.py:function1", + "-m", "cyclomatic.complexity", "-o", "test_granular.html", ], ) assert result.exit_code == 0, result.stdout + + +def test_graph_multiple_paths(builddir): + """Test the graph feature with multiple paths""" + runner = CliRunner() + with patch.dict("os.environ", values=PATCHED_ENV, clear=True): + result = runner.invoke( + main.cli, + ["--path", builddir, "graph", _path, "src/", "path3", "-m", "raw.loc"], + ) + assert result.exit_code == 0, result.stdout From 024c5946143bc8635552253e115e862c05bba44e Mon Sep 17 00:00:00 2001 From: devdanzin <74280297+devdanzin@users.noreply.github.com> Date: Tue, 15 Aug 2023 17:40:45 -0300 Subject: [PATCH 3/6] Allow passing files and directories together, which will include all files in directory and subdirectories. --- src/wily/commands/graph.py | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/wily/commands/graph.py b/src/wily/commands/graph.py index c6e966d8..263909bd 100644 --- a/src/wily/commands/graph.py +++ b/src/wily/commands/graph.py @@ -1,11 +1,11 @@ """ -Draw graph in HTML for a specific metric. +Graph command. -TODO: Add multiple lines for multiple files +Draw graph in HTML for a specific metric. """ from pathlib import Path -from typing import Optional +from typing import Optional, Tuple import plotly.graph_objs as go import plotly.offline @@ -22,9 +22,16 @@ def metric_parts(metric): return operator.name, met.name +def path_startswith(filename: str, path: str) -> bool: + """Check whether a filename starts with a given path in platform-agnostic way.""" + filepath = Path(filename).resolve() + path_ = Path(path).resolve() + return str(filepath).startswith(str(path_)) + + def graph( config: WilyConfig, - path: str, + path: Tuple[str, ...], metrics: str, output: Optional[str] = None, x_axis: Optional[str] = None, @@ -64,11 +71,14 @@ def graph( tracked_files = set() for rev in state.index[state.default_archiver].revisions: tracked_files.update(rev.revision.tracked_files) - paths = tuple( - tracked_file - for tracked_file in tracked_files - if any(tracked_file.startswith(p) for p in path) - ) or path + paths = ( + tuple( + tracked_file + for tracked_file in tracked_files + if any(path_startswith(tracked_file, p) for p in path) + ) + or path + ) else: paths = path operator, key = metric_parts(metrics[0]) From 92759faa82fd0ed4fd3fcb528103d981994cf057 Mon Sep 17 00:00:00 2001 From: devdanzin <74280297+devdanzin@users.noreply.github.com> Date: Tue, 15 Aug 2023 18:04:35 -0300 Subject: [PATCH 4/6] Give up trying to output all file names in graph(), but still display name if only one path was passed. --- src/wily/commands/graph.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/wily/commands/graph.py b/src/wily/commands/graph.py index 263909bd..f03bca30 100644 --- a/src/wily/commands/graph.py +++ b/src/wily/commands/graph.py @@ -65,7 +65,6 @@ def graph( metrics = metrics.split(",") y_metric = resolve_metric(metrics[0]) - title = f"{x_axis.capitalize()} of {y_metric.description} for {', '.join(path)}{' aggregated' if aggregate else ''}" if not aggregate: tracked_files = set() @@ -81,6 +80,11 @@ def graph( ) else: paths = path + + title = ( + f"{x_axis.capitalize()} of {y_metric.description}" + f"{(' for ' + paths[0]) if len(paths) == 1 else ''}{' aggregated' if aggregate else ''}" + ) operator, key = metric_parts(metrics[0]) if len(metrics) == 1: # only y-axis z_axis = z_operator = z_key = "" From 15ccc1e092b8b1a401b6e067ad2c7975a3a9a9dd Mon Sep 17 00:00:00 2001 From: devdanzin <74280297+devdanzin@users.noreply.github.com> Date: Tue, 15 Aug 2023 23:44:42 -0300 Subject: [PATCH 5/6] Rename metrics -> metrics_list to avoid typing error. --- src/wily/commands/graph.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/wily/commands/graph.py b/src/wily/commands/graph.py index f03bca30..7e417574 100644 --- a/src/wily/commands/graph.py +++ b/src/wily/commands/graph.py @@ -62,9 +62,9 @@ def graph( else: x_operator, x_key = metric_parts(x_axis) - metrics = metrics.split(",") + metrics_list = metrics.split(",") - y_metric = resolve_metric(metrics[0]) + y_metric = resolve_metric(metrics_list[0]) if not aggregate: tracked_files = set() @@ -85,12 +85,12 @@ def graph( f"{x_axis.capitalize()} of {y_metric.description}" f"{(' for ' + paths[0]) if len(paths) == 1 else ''}{' aggregated' if aggregate else ''}" ) - operator, key = metric_parts(metrics[0]) - if len(metrics) == 1: # only y-axis + operator, key = metric_parts(metrics_list[0]) + if len(metrics_list) == 1: # only y-axis z_axis = z_operator = z_key = "" else: - z_axis = resolve_metric(metrics[1]) - z_operator, z_key = metric_parts(metrics[1]) + z_axis = resolve_metric(metrics_list[1]) + z_operator, z_key = metric_parts(metrics_list[1]) for path_ in paths: current_path = str(Path(path_)) x = [] From b528da0558d7ab4217e18255705182dae0497afe Mon Sep 17 00:00:00 2001 From: devdanzin <74280297+devdanzin@users.noreply.github.com> Date: Tue, 15 Aug 2023 23:56:41 -0300 Subject: [PATCH 6/6] Fix tests, add new test for graph with multiple paths. --- test/unit/test_cli.py | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/test/unit/test_cli.py b/test/unit/test_cli.py index 0300c671..40c1f8a0 100644 --- a/test/unit/test_cli.py +++ b/test/unit/test_cli.py @@ -287,12 +287,32 @@ def test_graph(): with patch("wily.__main__.exists", return_value=True) as check_cache: with patch("wily.commands.graph.graph") as graph: runner = CliRunner() - result = runner.invoke(main.cli, ["graph", "foo.py", "example_metric"]) + result = runner.invoke( + main.cli, ["graph", "foo.py", "-m", "example_metric"] + ) + assert result.exit_code == 0 + assert graph.called_once + assert check_cache.called_once + assert graph.call_args[1]["path"] == ("foo.py",) + assert graph.call_args[1]["metrics"] == "example_metric" + + +def test_graph_multiple_paths(): + """ + Test that graph calls the graph command with multiple paths + """ + with patch("wily.__main__.exists", return_value=True) as check_cache: + with patch("wily.commands.graph.graph") as graph: + runner = CliRunner() + result = runner.invoke( + main.cli, + ["graph", "foo.py", "bar.py", "baz.py", "-m", "example_metric"], + ) assert result.exit_code == 0 assert graph.called_once assert check_cache.called_once - assert graph.call_args[1]["path"] == "foo.py" - assert graph.call_args[1]["metrics"] == ("example_metric",) + assert graph.call_args[1]["path"] == ("foo.py", "bar.py", "baz.py") + assert graph.call_args[1]["metrics"] == "example_metric" def test_graph_multiple_metrics(): @@ -303,13 +323,13 @@ def test_graph_multiple_metrics(): with patch("wily.commands.graph.graph") as graph: runner = CliRunner() result = runner.invoke( - main.cli, ["graph", "foo.py", "example_metric", "another_metric"] + main.cli, ["graph", "foo.py", "-m", "example_metric,another_metric"] ) assert result.exit_code == 0 assert graph.called_once assert check_cache.called_once - assert graph.call_args[1]["path"] == "foo.py" - assert graph.call_args[1]["metrics"] == ("example_metric", "another_metric") + assert graph.call_args[1]["path"] == ("foo.py",) + assert graph.call_args[1]["metrics"] == "example_metric,another_metric" def test_graph_with_output(): @@ -320,13 +340,13 @@ def test_graph_with_output(): with patch("wily.commands.graph.graph") as graph: runner = CliRunner() result = runner.invoke( - main.cli, ["graph", "foo.py", "example_metric", "-o", "foo.html"] + main.cli, ["graph", "foo.py", "-m", "example_metric", "-o", "foo.html"] ) assert result.exit_code == 0 assert graph.called_once assert check_cache.called_once - assert graph.call_args[1]["path"] == "foo.py" - assert graph.call_args[1]["metrics"] == ("example_metric",) + assert graph.call_args[1]["path"] == ("foo.py",) + assert graph.call_args[1]["metrics"] == "example_metric" assert graph.call_args[1]["output"] == "foo.html"