Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[issue-586] fix CLI #588

Merged
merged 2 commits into from
Apr 17, 2023
Merged

[issue-586] fix CLI #588

merged 2 commits into from
Apr 17, 2023

Conversation

meretp
Copy link
Collaborator

@meretp meretp commented Apr 17, 2023

With this PR we make infile a required argument to display a help message when no arguments are given.
I also added some test to check that the cli fails for invalid arguments and passes for correct arguments.

fixes #586

Copy link
Collaborator

@armintaenzertng armintaenzertng left a comment

Choose a reason for hiding this comment

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

Thanks for the addition :)

("-i", os.path.join(os.path.dirname(__file__), "data/SPDXJSONExample-v2.3.spdx.json"), "-o", "-"),
],
)
def test_cli(options: Tuple[str, str]):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure about this type hint, as there are different numbers of strings in the passed tuples. In the test below there is no type hint, either, so maybe just omit it...?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah sure, that was a blind copy and paste mistake

("-i", os.path.join(os.path.dirname(__file__), "data/SPDXJSONExample-v2.3.spdx.json"), "-o"),
],
)
def test_cli_with_system_exit(options):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, one other thing; every CLI call has a system exit, the important part here is the exit code:

Suggested change
def test_cli_with_system_exit(options):
def test_cli_with_system_exit_code_2(options):

Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe also adapt the other test name to be consistent

@meretp meretp force-pushed the fix-cli branch 2 times, most recently from fcceed9 to e9886ae Compare April 17, 2023 12:16
meretp added 2 commits April 17, 2023 14:25
… when no arguments are given

Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>
Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>
@meretp meretp merged commit 3204bda into spdx:main Apr 17, 2023
@meretp meretp deleted the fix-cli branch April 17, 2023 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.

CLI raises error if no arguments provided
2 participants