Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #135 +/- ##
==========================================
+ Coverage 92.78% 93.47% +0.68%
==========================================
Files 6 6
Lines 929 996 +67
==========================================
+ Hits 862 931 +69
+ Misses 67 65 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Reviewer's GuideAdds a new GitLab Code Quality formatter and supporting formatter architecture changes (metadata-aware error/warning signatures, finalize hook, and OutputFormat extension), wires finalize into CLI lifecycle, and updates tests and docs to cover GitLab reports, formatter behavior, and project conventions while bumping the project version. Sequence diagram for check command with GitLab accumulating formatter and finalize hooksequenceDiagram
actor User
participant CLI as cli.main
participant Core as core.check
participant Factory as get_formatter
participant Fmt as GitlabFormatter
participant GitlabReport as gl-code-quality-report.json
User->>CLI: ruff-sync check --output-format gitlab
CLI->>Factory: get_formatter(OutputFormat.GITLAB)
Factory-->>CLI: GitlabFormatter instance
CLI->>Core: await check(args, fmt=Fmt)
activate Core
Core->>Fmt: note("🔍 Checking Ruff sync status...")
Core->>Core: resolve_target_path()
Core->>Core: load and merge upstream configs
alt configuration in sync
Core->>Fmt: success("✅ Ruff configuration is in sync.")
Core-->>CLI: return 0
else configuration drift detected
Core->>Fmt: error(message, file_path, logger, check_name, drift_key)
opt --diff enabled
Core->>Fmt: diff(diff_text)
end
Core-->>CLI: return 1
end
deactivate Core
CLI->>Fmt: finalize()
activate Fmt
Fmt->>GitlabReport: print JSON array of GitlabIssue objects to stdout
deactivate Fmt
CLI-->>User: exit code (0 when in sync, 1 on drift)
Class diagram for updated formatter architecture with GitLab supportclassDiagram
class ResultFormatter {
<<protocol>>
+note(message: str) void
+info(message: str, logger: logging.Logger|None) void
+success(message: str) void
+error(message: str, file_path: pathlib.Path|None, logger: logging.Logger|None, check_name: str, drift_key: str|None) void
+warning(message: str, file_path: pathlib.Path|None, logger: logging.Logger|None, check_name: str, drift_key: str|None) void
+debug(message: str, logger: logging.Logger|None) void
+diff(diff_text: str) void
+finalize() void
}
class TextFormatter {
+note(message: str) void
+info(message: str, logger: logging.Logger|None) void
+success(message: str) void
+error(message: str, file_path: pathlib.Path|None, logger: logging.Logger|None, check_name: str, drift_key: str|None) void
+warning(message: str, file_path: pathlib.Path|None, logger: logging.Logger|None, check_name: str, drift_key: str|None) void
+debug(message: str, logger: logging.Logger|None) void
+diff(diff_text: str) void
+finalize() void
}
class GithubFormatter {
+note(message: str) void
+info(message: str, logger: logging.Logger|None) void
+success(message: str) void
+error(message: str, file_path: pathlib.Path|None, logger: logging.Logger|None, check_name: str, drift_key: str|None) void
+warning(message: str, file_path: pathlib.Path|None, logger: logging.Logger|None, check_name: str, drift_key: str|None) void
+debug(message: str, logger: logging.Logger|None) void
+diff(diff_text: str) void
+finalize() void
-_escape(value: str, is_property: bool) str
}
class JsonFormatter {
+note(message: str) void
+info(message: str, logger: logging.Logger|None) void
+success(message: str) void
+error(message: str, file_path: pathlib.Path|None, logger: logging.Logger|None, check_name: str, drift_key: str|None) void
+warning(message: str, file_path: pathlib.Path|None, logger: logging.Logger|None, check_name: str, drift_key: str|None) void
+debug(message: str, logger: logging.Logger|None) void
+diff(diff_text: str) void
+finalize() void
}
class GitlabLines {
<<TypedDict>>
+begin: int
}
class GitlabLocation {
<<TypedDict>>
+path: str
+lines: GitlabLines
}
class GitlabIssue {
<<TypedDict>>
+description: str
+check_name: str
+fingerprint: str
+severity: Literal
+location: GitlabLocation
}
class GitlabFormatter {
-_issues: list[GitlabIssue]
+GitlabFormatter()
+note(message: str) void
+info(message: str, logger: logging.Logger|None) void
+success(message: str) void
+error(message: str, file_path: pathlib.Path|None, logger: logging.Logger|None, check_name: str, drift_key: str|None) void
+warning(message: str, file_path: pathlib.Path|None, logger: logging.Logger|None, check_name: str, drift_key: str|None) void
+debug(message: str, logger: logging.Logger|None) void
+diff(diff_text: str) void
+finalize() void
-_make_issue(description: str, check_name: str, severity: Literal, file_path: pathlib.Path|None, drift_key: str|None) GitlabIssue
-_make_fingerprint(path: str, drift_key: str|None) str
}
class OutputFormat {
<<enum>>
TEXT
JSON
GITHUB
GITLAB
}
class FormatterFactory {
+get_formatter(output_format: OutputFormat) ResultFormatter
}
ResultFormatter <|.. TextFormatter
ResultFormatter <|.. GithubFormatter
ResultFormatter <|.. JsonFormatter
ResultFormatter <|.. GitlabFormatter
GitlabFormatter o--> GitlabIssue
GitlabIssue o--> GitlabLocation
GitlabLocation o--> GitlabLines
OutputFormat ..> FormatterFactory
FormatterFactory ..> TextFormatter
FormatterFactory ..> GithubFormatter
FormatterFactory ..> JsonFormatter
FormatterFactory ..> GitlabFormatter
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
GitlabFormatter._make_issue,location.pathis documented as needing to be relative to the repo root but currently usesstr(file_path)directly, which may be absolute; consider normalizing this to a relative path (e.g., viaPath.cwd()or a passed-in repo root) to avoid GitLab failing to link annotations. - The
JsonFormatter.error/warningmethods now acceptcheck_namebut ignore it in the emitted JSON; ifcheck_nameis part of the structured metadata contract, consider adding it to the JSON payload for consistency with other structured outputs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `GitlabFormatter._make_issue`, `location.path` is documented as needing to be relative to the repo root but currently uses `str(file_path)` directly, which may be absolute; consider normalizing this to a relative path (e.g., via `Path.cwd()` or a passed-in repo root) to avoid GitLab failing to link annotations.
- The `JsonFormatter.error`/`warning` methods now accept `check_name` but ignore it in the emitted JSON; if `check_name` is part of the structured metadata contract, consider adding it to the JSON payload for consistency with other structured outputs.
## Individual Comments
### Comment 1
<location path="src/ruff_sync/formatters.py" line_range="437" />
<code_context>
+ file_path: pathlib.Path | None,
+ fingerprint: str | None,
+ ) -> dict[str, Any]:
+ path = str(file_path) if file_path else "pyproject.toml"
+ fp = fingerprint or self._auto_fingerprint(description, path)
+ return {
</code_context>
<issue_to_address>
**issue (bug_risk):** Normalize GitLab issue paths to be relative to repo root to match GitLab expectations
`location.path` in the Code Quality spec must be relative to the project root, but `file_path` here can be absolute (e.g. when `pull()` passes `_source_toml_path`). That would violate the spec and may cause GitLab to misplace or ignore annotations. Please normalize to a project-root-relative path (e.g. `Path.cwd()`-relative as in `check()`) or ensure all callers pass a relative path.
</issue_to_address>
### Comment 2
<location path="tests/test_formatters.py" line_range="157" />
<code_context>
assert data["file"] == "f.py"
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for JsonFormatter’s new drift_key (and optionally check_name) metadata fields.
The formatter now supports `drift_key` (and accepts `check_name`), but the tests only assert on `file`. Please extend `test_metadata_propagation` (or add a sibling test) to invoke the formatter with both `file_path` and `drift_key` set, and assert that the JSON includes `drift_key` with the expected value. If `check_name` is meant to appear in the JSON, add an assertion for that as well so schema changes are caught by tests.
Suggested implementation:
```python
assert data["file"] == "f.py"
assert data["drift_key"] == "drift-123"
assert data.get("check_name") == "example-check"
```
To fully implement the suggestion, also ensure that the test invoking the `JsonFormatter` passes the matching metadata values:
1. Wherever `JsonFormatter` (or the helper that exercises it in `test_metadata_propagation`) is called, update the call to include `drift_key="drift-123"` and `check_name="example-check"` alongside `file_path="f.py"`. For example, if the current call looks like:
```python
formatter = JsonFormatter(stream=io.StringIO())
formatter.format(result, file_path="f.py")
```
adjust it to:
```python
formatter = JsonFormatter(stream=io.StringIO())
formatter.format(
result,
file_path="f.py",
drift_key="drift-123",
check_name="example-check",
)
```
2. If `test_metadata_propagation` also exercises non-JSON formatters, either:
* restrict the new `drift_key` / `check_name` assertions to the JSON case (e.g., by only running them when `formatter` is a `JsonFormatter`), or
* move this logic into a JSON-specific sibling test such as `test_json_metadata_propagation_includes_drift_key_and_check_name`.
</issue_to_address>
### Comment 3
<location path="tests/test_formatters.py" line_range="160" />
<code_context>
assert data["file"] == "f.py"
+class TestGitlabFormatter:
+ """Tests for GitlabFormatter (GitLab Code Quality report format)."""
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test for the GitLab formatter’s default path behaviour when file_path is None.
Currently, all tests pass a file_path. Please also cover the config-level case where `file_path` is `None` and `GitlabFormatter._make_issue` should set `location.path` to `"pyproject.toml"` (e.g. `fmt.error("drift", file_path=None)` and assert `issue["location"]["path"] == "pyproject.toml"`). This will lock in the default-path behavior the formatter depends on.
Suggested implementation:
```python
class TestGitlabFormatter:
"""Tests for GitlabFormatter (GitLab Code Quality report format)."""
def test_default_path_when_file_path_is_none(self) -> None:
"""When file_path is None, GitlabFormatter should default to 'pyproject.toml'."""
output = io.StringIO()
fmt = GitlabFormatter(output)
# Trigger an issue without an explicit file_path: formatter should fall back
# to its default config file path ("pyproject.toml").
fmt.error("drift", file_path=None)
fmt.finalize()
output.seek(0)
issues = json.loads(output.getvalue() or "[]")
assert issues, "Expected at least one issue to be emitted"
assert issues[0]["location"]["path"] == "pyproject.toml"
```
1. Ensure `io` and `json` are imported at the top of `tests/test_formatters.py`:
- If not already present, add `import io` and `import json` alongside the other imports.
2. If `GitlabFormatter` requires additional constructor arguments (e.g. config/path), update the instantiation in the test accordingly so that it uses the same pattern as the other GitLab formatter tests in this file.
3. If `fmt.error` has a different signature (e.g. requires a message or other parameters), adjust the call in the test to match the existing usage elsewhere in the test suite, keeping `file_path=None` to exercise the default-path behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Owner
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/test_formatters.py" line_range="205-207" />
<code_context>
+ issues = json.loads(capsys.readouterr().out)
+ assert issues == []
+
+ def test_error_produces_major_severity(self, capsys: pytest.CaptureFixture[str]) -> None:
+ """error() must accumulate a major-severity issue."""
+ fmt = GitlabFormatter()
+ fmt.error("drift found", file_path=pathlib.Path("pyproject.toml"))
+ fmt.finalize()
+ issues = json.loads(capsys.readouterr().out)
+ assert len(issues) == 1
+ assert issues[0]["severity"] == "major"
+ assert issues[0]["location"]["path"] == "pyproject.toml"
+ assert issues[0]["location"]["lines"]["begin"] == 1
+ assert "fingerprint" in issues[0]
+ assert "description" in issues[0]
+ assert "check_name" in issues[0]
+
+ def test_warning_produces_minor_severity(self, capsys: pytest.CaptureFixture[str]) -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Assert that GitLab formatter uses the expected default check_name when not overridden
Since `check_name` is part of the external contract, it’d be helpful to also assert its exact value (e.g. `"ruff-sync/config-drift"`). That way the test will catch accidental changes to the default identifier that could affect downstream consumers.
```suggestion
assert "fingerprint" in issues[0]
assert "description" in issues[0]
# Default check_name is part of the external contract; assert exact value
assert issues[0]["check_name"] == "ruff-sync/config-drift"
```
</issue_to_address>
### Comment 2
<location path="tests/test_formatters.py" line_range="27-32" />
<code_context>
+# ---------------------------------------------------------------------------
+
@pytest.fixture(params=[TextFormatter, GithubFormatter, JsonFormatter])
def formatter(request: pytest.FixtureRequest) -> ResultFormatter:
- """Fixture providing instances of all registered formatters."""
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests that verify finalize() is invoked in CLI workflows even on early returns
Current coverage only ensures `finalize()` exists and is a no-op for streaming formatters, and that GitLab’s accumulating formatter uses it in normal flows. Since `check` and `pull` now always wrap formatter usage in `try/finally`, please add a CLI-focused test that injects a spy formatter via `get_formatter` and asserts `finalize()` is called on both the success path and early-return error paths (e.g., missing config). This will lock in the guarantee that finalization always runs, which is particularly important for accumulating formatters like GitLab.
Suggested implementation:
```python
@pytest.fixture(params=[TextFormatter, GithubFormatter, JsonFormatter])
def formatter(request: pytest.FixtureRequest) -> ResultFormatter:
"""Fixture providing instances of all streaming formatters."""
formatter_cls: type[ResultFormatter] = request.param
return formatter_cls()
class SpyFormatter(TextFormatter):
"""Formatter spy that records whether finalize() was called."""
def __init__(self, *args, **kwargs) -> None: # type: ignore[no-untyped-def]
super().__init__(*args, **kwargs)
self.finalized_called = False
def finalize(self) -> None:
self.finalized_called = True
@pytest.mark.usefixtures("tmp_path")
def test_cli_check_calls_finalize_on_success(monkeypatch: pytest.MonkeyPatch, tmp_path: "Path") -> None:
"""Ensure finalize() is called in the CLI check happy path."""
spy = SpyFormatter()
def fake_get_formatter(*args, **kwargs): # type: ignore[no-untyped-def]
return spy
# Route all formatter creation in the check CLI through our spy.
monkeypatch.setattr(
"ruff_sync.formatters.get_formatter",
fake_get_formatter,
raising=True,
)
# Import locally to avoid import cycles at module import time.
from ruff_sync.cli import check as check_cli # type: ignore[import]
# Arrange: create a minimal valid configuration so that the command succeeds
config_path = tmp_path / "pyproject.toml"
config_path.write_text(
"[tool.ruff-sync]\n"
"upstream = \"origin/main\"\n",
encoding="utf-8",
)
# Act: run the CLI check command in a way that exercises the try/finally block.
check_cli.main(["--config", str(config_path)])
# Assert: finalize() has been invoked on the spy formatter.
assert spy.finalized_called is True
@pytest.mark.usefixtures("tmp_path")
def test_cli_check_calls_finalize_on_early_return(monkeypatch: pytest.MonkeyPatch, tmp_path: "Path") -> None:
"""Ensure finalize() is called even when check exits early (e.g. missing/invalid config)."""
spy = SpyFormatter()
def fake_get_formatter(*args, **kwargs): # type: ignore[no-untyped-def]
return spy
monkeypatch.setattr(
"ruff_sync.formatters.get_formatter",
fake_get_formatter,
raising=True,
)
from ruff_sync.cli import check as check_cli # type: ignore[import]
# Arrange: point to a config path that does not exist to trigger an early-return error path.
missing_config_path = tmp_path / "does-not-exist.toml"
# Act: run the CLI, expecting it to exit non‑zero (e.g. via SystemExit).
with pytest.raises(SystemExit):
check_cli.main(["--config", str(missing_config_path)])
# Assert: even on early exit, finalize() was still called.
assert spy.finalized_called is True
```
These edits assume:
1. There is a `ruff_sync.cli.check` module exposing a `main(argv: list[str])` function that:
- Uses `ruff_sync.formatters.get_formatter(...)` to construct the formatter.
- Wraps formatter usage in a `try/finally` block that always calls `formatter.finalize()`.
2. The CLI accepts a `--config PATH` option and:
- Succeeds when the pointed `pyproject.toml` exists and is valid.
- Raises `SystemExit` (or otherwise terminates) when the config is missing/invalid.
If your actual CLI surface differs, you will need to:
1. Adjust the imports in both tests:
- Replace `from ruff_sync.cli import check as check_cli` with the correct module / symbol.
- If the entrypoint is named differently (e.g. `cli`, `run`, `check`), update `check_cli.main([...])` accordingly.
2. Adjust the monkeypatch target if `get_formatter` is imported into the CLI module rather than accessed via `ruff_sync.formatters`:
- For example, if `check.py` does `from ruff_sync.formatters import get_formatter`, change:
- `"ruff_sync.formatters.get_formatter"` → `"ruff_sync.cli.check.get_formatter"`.
3. Adapt the arguments to the CLI invocation if `--config` is not the correct flag or the config mechanism differs:
- E.g. `["--config-file", str(config_path)]` or other appropriate flags.
4. If `SystemExit` is not raised on missing config but an error code is returned instead:
- Replace the `with pytest.raises(SystemExit): ...` block with a direct call and assertion on the return code.
5. Ensure `Path` is imported or available in `tests/test_formatters.py`:
- If not already present at the top of the file, add:
- `from pathlib import Path`.
With these small adjustments to align with your real CLI API, the tests will guarantee that `finalize()` is invoked in both the success and early‑error code paths of the CLI, protecting accumulating formatters like GitLab.
</issue_to_address>
### Comment 3
<location path=".agents/formatters-architecture.md" line_range="51" />
<code_context>
+| `warning(message, file_path, logger, check_name, drift_key)` | Non-fatal issue (e.g. stale pre-commit hook) | See §Semantic Fields |
+| `debug(message, logger)` | Verbose internal state | Streaming |
+| `diff(diff_text)` | Unified diff between upstream and local | Intentionally ignored by structured formatters |
+| `finalize()` | **Always called in a `try…finally` by `cli.py`** | No-op for streaming; writes report for accumulating |
+
+### Semantic Fields on `error()` and `warning()`
</code_context>
<issue_to_address>
**issue:** Clarify `finalize()` contract to avoid conflict with `.agents/gitlab-reports.md` guidance.
This description says `finalize()` is always on the `ResultFormatter` protocol and always called from `cli.py`, whereas `.agents/gitlab-reports.md` still allows for formatters without `finalize()` (using `hasattr`/`isinstance`). Please align these so there’s a single, clear contract for `finalize()` (for example, document that it is required and older optional patterns are deprecated), to avoid confusing formatter implementers.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Owner
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
GitlabFormatter._make_fingerprint, the fingerprint is derived only frompathanddrift_key; if you later introduce multiple check types at the same path/key (e.g., differentcheck_namevalues), they will collide, so consider includingcheck_name(and possibly upstream identity) in the fingerprint input to keep issues distinct. - For
GitlabFormatter._make_issue, whenfile_pathis absolute but not undercwd, you fall back to the raw absolute string even though GitLab expects paths relative to the repo root; it may be safer to normalize such cases (e.g., always emit just the filename or a best-effort relative path) to avoid losing annotations.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `GitlabFormatter._make_fingerprint`, the fingerprint is derived only from `path` and `drift_key`; if you later introduce multiple check types at the same path/key (e.g., different `check_name` values), they will collide, so consider including `check_name` (and possibly upstream identity) in the fingerprint input to keep issues distinct.
- For `GitlabFormatter._make_issue`, when `file_path` is absolute but not under `cwd`, you fall back to the raw absolute string even though GitLab expects paths relative to the repo root; it may be safer to normalize such cases (e.g., always emit just the filename or a best-effort relative path) to avoid losing annotations.
## Individual Comments
### Comment 1
<location path="src/ruff_sync/formatters.py" line_range="450-453" />
<code_context>
+ ) -> dict[str, Any]:
+ path = str(file_path) if file_path else "pyproject.toml"
+ fp = fingerprint or self._auto_fingerprint(description, path)
+ return {
+ "description": description,
+ "check_name": check_name,
</code_context>
<issue_to_address>
**issue (bug_risk):** GitLab issue fingerprints only use path and drift key, which risks collisions between distinct issues.
Because the fingerprint only uses `path` and `drift_key`, distinct issues with the same pair (for example, different `check_name`s or messages) will be merged by GitLab, breaking introduced/resolved tracking. Please include `check_name` (and optionally a normalized description) in the fingerprint input so different rules on the same key generate distinct, deterministic fingerprints (for example, `raw = f"ruff-sync:{check_name}:{path}:{drift_key or ''}"`).
</issue_to_address>
### Comment 2
<location path="tests/test_formatters.py" line_range="226-235" />
<code_context>
+ def test_drift_key_produces_distinct_fingerprints(
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a GitLab fingerprint test for cases without drift_key
Current tests cover distinct fingerprints for different `drift_key` values and stability when both `path` and `drift_key` match, but not the fallback behavior when `drift_key` is omitted. Since the implementation then uses `path` only, please add a test that:
- calls `error()` twice with the same `file_path` and `drift_key=None`, asserting equal fingerprints
- optionally calls `error()` with different `file_path` values (no `drift_key`), asserting different fingerprints
This will lock in behavior for tools that don’t pass a `drift_key` while keeping fingerprints stable and distinct.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
SourceryAI
reviewed
Mar 30, 2026
SourceryAI
left a comment
SourceryAI
left a comment
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/test_formatters.py" line_range="164-173" />
<code_context>
+class TestGitlabFormatter:
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test for absolute paths that are *not* under the current working directory.
Right now you only test absolute paths under `cwd` that normalize to relative paths. Please also cover the case where `file_path.is_absolute()` is true but `relative_to(pathlib.Path.cwd())` raises `ValueError` (e.g. different drive or outside the repo), and assert that `location.path` matches the intended fallback (likely the original string). This will lock in the expected behavior for that branch.
Suggested implementation:
```python
class TestGitlabFormatter:
"""Tests for GitlabFormatter (GitLab Code Quality report format)."""
def test_default_path_when_file_path_is_none(self, capsys: pytest.CaptureFixture[str]) -> None:
"""When file_path is None, GitlabFormatter should default to 'pyproject.toml'."""
fmt = GitlabFormatter()
fmt.error("drift", file_path=None)
fmt.finalize()
issues = json.loads(capsys.readouterr().out)
assert len(issues) == 1
assert issues[0]["location"]["path"] == "pyproject.toml"
def test_absolute_path_outside_cwd_falls_back_to_original_string(
self,
capsys: pytest.CaptureFixture[str],
) -> None:
"""Absolute paths not under cwd should be preserved as-is in location.path."""
fmt = GitlabFormatter()
# Construct an absolute path that is not a sub-path of the current working directory
outside_path = (pathlib.Path.cwd().parent / "outside_project" / "some_file.py").resolve()
fmt.error("drift", file_path=outside_path)
fmt.finalize()
issues = json.loads(capsys.readouterr().out)
assert len(issues) == 1
# When relative_to(Path.cwd()) raises ValueError, GitlabFormatter should fall back
# to using the original absolute path string.
assert issues[0]["location"]["path"] == str(outside_path)
```
1. Ensure `pathlib` is imported at the top of `tests/test_formatters.py`:
- If not already present, add `import pathlib` alongside the other imports.
2. This test assumes that `GitlabFormatter` implements the branch where an absolute `file_path`
that is not under `Path.cwd()` is preserved as the original string when `relative_to` fails.
If the current implementation differs, adjust the expected assertion to match the actual fallback behavior.
</issue_to_address>Hi @Kilo59! 👋
Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀
Install the sourcery-ai bot to get automatic code reviews on every pull request ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add a GitLab Code Quality JSON report formatter, integrate it into the formatter pipeline and CLI lifecycle, and document the formatter architecture and GitLab CI usage while tightening testing and project guidelines.
New Features:
Enhancements:
Documentation:
Tests:
Chores: