Skip to content

Commit

Permalink
chore: fix secureli version bug in non secureli repo (#467)
Browse files Browse the repository at this point in the history
secureli-466

<!-- Include general description here -->
Fix for issue where secureli --version errors out when not run in a repo
that has secureli initialized.

## Changes
<!-- A detailed list of changes -->
* Fixed bug

## Testing
<!--
Mention updated tests and any manual testing performed.
Are aspects not yet tested or not easily testable?
Feel free to include screenshots if appropriate.
 -->
* Added unit tests
* All existing unit tests passing

## Clean Code Checklist
<!-- This is here to support you. Some/most checkboxes may not apply to
your change -->
- [x] Meets acceptance criteria for issue
- [x] New logic is covered with automated tests
- [ ] Appropriate exception handling added
- [ ] Thoughtful logging included
- [ ] Documentation is updated
- [ ] Follow-up work is documented in TODOs
- [ ] TODOs have a ticket associated with them
- [x] No commented-out code included


<!--
Github-flavored markdown reference:
https://docs.github.com/en/get-started/writing-on-github
-->
  • Loading branch information
isaac-heist-slalom authored Feb 27, 2024
1 parent 8d9e503 commit f25712d
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 13 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ repos:
- id: check-yaml
- id: check-added-large-files
- repo: https://github.com/psf/black-pre-commit-mirror
rev: 24.1.1
rev: 24.2.0
hooks:
- id: black
- repo: https://github.com/yelp/detect-secrets
Expand Down
8 changes: 8 additions & 0 deletions secureli/abstractions/pre_commit.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,3 +304,11 @@ def get_pre_commit_config(self, folder_path: Path):
contents = f.read()
yaml_values = yaml.safe_load(contents)
return PreCommitSettings(**yaml_values)

def pre_commit_config_exists(self, folder_path: Path) -> bool:
"""
Checks if the .pre-commit-config file exists and returns a boolean
:return: boolean - True if config exists and False if not
"""
path_to_config = folder_path / ".pre-commit-config.yaml"
return path_to_config.exists()
26 changes: 15 additions & 11 deletions secureli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,24 @@
def version_callback(value: bool):
if value:
typer.echo(secureli_version())
typer.echo("\nHook Versions:")
typer.echo("--------------")
config = container.pre_commit_abstraction().get_pre_commit_config(Path("."))
pre_commit_abstr = container.pre_commit_abstraction()
path = Path(".")

all_repos = [
(hook.id, repo.rev.lstrip("v"))
for repo in config.repos
for hook in repo.hooks
]
if pre_commit_abstr.pre_commit_config_exists(path):
typer.echo("\nHook Versions:")
typer.echo("--------------")
config = pre_commit_abstr.get_pre_commit_config(path)

sorted_repos = sorted(all_repos, key=lambda x: x[0])
all_repos = [
(hook.id, repo.rev.lstrip("v"))
for repo in config.repos
for hook in repo.hooks
]

for hook_id, version in sorted_repos:
typer.echo(f"{hook_id:<30} {version}")
sorted_repos = sorted(all_repos, key=lambda x: x[0])

for hook_id, version in sorted_repos:
typer.echo(f"{hook_id:<30} {version}")

raise typer.Exit()

Expand Down
12 changes: 12 additions & 0 deletions tests/abstractions/test_pre_commit.py
Original file line number Diff line number Diff line change
Expand Up @@ -469,3 +469,15 @@ def test_check_for_hook_updates_returns_repos_with_new_revs(
assert len(updated_repos) == 1 # only the first repo should be returned
assert updated_repos[repo_urls[0]].oldRev == "tag1"
assert updated_repos[repo_urls[0]].newRev == "tag2"


def test_pre_commit_config_exists(pre_commit: PreCommitAbstraction):
with um.patch.object(Path, "exists", return_value=True):
pre_commit_exists = pre_commit.pre_commit_config_exists(test_folder_path)
assert pre_commit_exists == True


def test_pre_commit_config_does_not_exist(pre_commit: PreCommitAbstraction):
with um.patch.object(Path, "exists", return_value=False):
pre_commit_exists = pre_commit.pre_commit_config_exists(test_folder_path)
assert pre_commit_exists == False
14 changes: 13 additions & 1 deletion tests/application/test_main.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from pathlib import Path
from unittest.mock import MagicMock
from unittest.mock import MagicMock, patch
from typer.testing import CliRunner

import pytest
Expand Down Expand Up @@ -63,6 +63,18 @@ def test_that_app_implements_version_option(
mock_container.wire.assert_not_called()


@pytest.mark.parametrize("test_input", ["-v", "--version"])
def test_that_version_callback_does_not_return_hook_versions_if_no_config(
test_input: str,
):
with patch.object(Path, "exists", return_value=False):
result = CliRunner().invoke(secureli.main.app, [test_input])

assert result.exit_code is 0
assert secureli_version() in result.stdout
assert "\nHook Versions:" not in result.stdout


def test_that_app_ignores_version_callback(mock_container: MagicMock):
result = CliRunner().invoke(secureli.main.app, ["scan"])

Expand Down

0 comments on commit f25712d

Please sign in to comment.