Skip to content

Refactor/remove prysk#87

Merged
ckunki merged 6 commits into
mainfrom
refactor/remove-prysk
Jun 2, 2026
Merged

Refactor/remove prysk#87
ckunki merged 6 commits into
mainfrom
refactor/remove-prysk

Conversation

@ckunki
Copy link
Copy Markdown
Contributor

@ckunki ckunki commented Jun 1, 2026

No description provided.

@ckunki ckunki temporarily deployed to manual-approval June 1, 2026 09:01 — with GitHub Actions Inactive
@ckunki ckunki deployed to manual-approval June 1, 2026 09:07 — with GitHub Actions Active
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 1, 2026

def test_parse_subcommand(command: list[str], sample_module: Path):
result = _run(command, "parse", sample_module.name, cwd=sample_module.parent)

assert result.stderr == ""
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can --debug cause any stderr output?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not find any reference to CLI option --debug except in _cli.py.
Hence, I don't assume CLI option --debug to cause any specific output.
However, with or without CLI option --debug I think, output to stderr is possible.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant the --debug in _run(...). But maybe I am lacking the context, so feel free to ignore it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My current guess is, that the CLI option --debug is used to prevent subprocess.run() to raise errors during integration tests.

Actually I think this is not optimal and could be avoided by using click.CliRunner().

I also see some more questionable things, e.g. ec_path = shutil.which("ec").

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests are not perfect, but better then prysk. For that reason, I suggest we merge them for the moment and we create a ticket to refactor the cli and make it easier testable.

Copy link
Copy Markdown
Contributor Author

@ckunki ckunki Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ckunki ckunki merged commit 1e81342 into main Jun 2, 2026
31 checks passed
@ckunki ckunki deleted the refactor/remove-prysk branch June 2, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants