fix: add description for subcommands #1120

marcosdotme · 2024-05-16T19:01:34Z

Description

Checklist

Add test cases to all the changes you introduce
Run ./scripts/format and ./scripts/test locally to ensure this change passes linter check and test
Test the changes on the local machine manually
Update the documentation for the changes (need remake a bunch of gifs/images @Lee-W)

Expected behavior

You must see the command description when running with --help option

Steps to Test This Pull Request

cz commit --help

Screenshot

marcosdotme · 2024-05-16T19:03:24Z

@Lee-W I delete the tests/commands/test_other_commands.py an move the tests for the correspondent file without asking you. Let me know if you need that I revert or squash this.

codecov · 2024-05-16T19:05:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.54%. Comparing base (120d514) to head (bbf19ad).
Report is 327 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1120      +/-   ##
==========================================
+ Coverage   97.33%   97.54%   +0.20%     
==========================================
  Files          42       55      +13     
  Lines        2104     2486     +382     
==========================================
+ Hits         2048     2425     +377     
- Misses         56       61       +5

Flag	Coverage Δ
unittests	`97.54% <100.00%> (+0.20%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

commitizen/cli.py

Lee-W · 2024-05-18T11:14:29Z

tests/commands/test_bump_command.py

+ cli.main()
+
+ out, _ = capsys.readouterr()
+ assert "bump semantic version based on the git log" in str(out).replace("\n", " ")


none blocker, but we could try something like

commitizen/tests/test_bump_update_version_in_files.py

Lines 157 to 158 in f1ece99

with open(multiple_versions_increase_string, encoding="utf-8") as f:

file_regression.check(f.read(), extension=".txt")

as well

I don't know if I understand... I wrote the test_bump_command_shows_description_when_use_help_option to assert that the command description shows on running --help against the command. Can you explain your idea with other words?

pytest-regeression is a tool that you can verify whether the content (e.g., str, data frame) is exactly the same as the last time we generated it. It done so by creating a text file that records the last result. https://pytest-regressions.readthedocs.io/en/latest/overview.html

I will look to this later, today. Thanks!

@Lee-W pytest-regressions is a game changer! Using the assert only approach isn't enough to check if the command descriptions matches. For example:

Using assert "create new commit" in str(out).replace("\n", " ") isn't reliable because if we add an dot to the command description "create new commit." it still matching.

But, using file_regression we can check for the entire output and adding the dot to the end of the command description will cause the test to fail.

I replaced all the assertion approach for the file_regression, except for two: changelog and bump.

Analyzing the failure I found the "problem" is in the file commitizen.version_schemes.py. Theres an constant called KNOWN_SCHEMES that is an set of all available scheme providers. The problem is: set is an unordered struct so the order changes for every call. Since the order changes for every call, we cannot assure using file_regression:

To fix this we can just replace the set comprehension for an list comprehension:

- KNOWN_SCHEMES = {ep.name for ep in metadata.entry_points(group=SCHEMES_ENTRYPOINT)} + KNOWN_SCHEMES = [ep.name for ep in metadata.entry_points(group=SCHEMES_ENTRYPOINT)]

I change the set for list and it passes on all tests, and now we can use file_regression. Theres an reason this to be an set? We can change to list?

@Lee-W I will wait to #1126 be merged because this branch will also fail on CI due to trailing whitespaces on *.svg

It's merged 🎉

@Lee-W After run the ./scripts/test it fails because the missing ":" on commit message:

The commit is:

How to proceed? 🥲

Maybe a rebase to edit the commit message?

oh just ignore it and push it. you can rebase from the master branch instead of merging. DO NOT edit the commit message though

…sion Set is an unordered structure, so every call the order changes. Because the unordered behavior we can't use `file_regression` to test the command output.

marcosdotme · 2024-05-20T18:38:37Z

@Lee-W Can you help me? Dont know whats happening here lol. All works fine on my local branch, all tests passes:

But it still failing on CI

Lee-W · 2024-05-20T18:39:55Z

@Lee-W Can you help me? Dont know whats happening here lol. All works fine on my local branch, all tests passes:

But it still failing on CI

Sure, let me take a look 🙂

Lee-W · 2024-05-20T18:41:25Z

I guess it's due to the latest main branch change. could you please run poetry run pytest --force-regen and see whether it fixes the CI? If not, I'll take a deeper look

marcosdotme · 2024-05-20T18:49:21Z

I guess it's due to the latest main branch change. could you please run poetry run pytest --force-regen and see whether it fixes the CI? If not, I'll take a deeper look

Tried poetry run pytest --force-regen but it fails and I think is because the pytes-xdist plugin:

So I try to put --force-regen directly on .scripts/test:

But nothing changed:

Lee-W · 2024-05-20T19:01:23Z

got it. let me take a look

Lee-W · 2024-05-20T19:08:30Z

I know what's happening. The message of argparse is different in different Python versions.

Lee-W · 2024-05-20T19:16:21Z

Changed in Python 3.10. We could probably generate two sets of expected results and run that test based on different Python versions. We could try https://docs.pytest.org/en/latest/how-to/skipping.html#id1. Not sure whether there's something like parameterize we can use in this case. @marcosdotme do you want me to resolve it? or do you want to take a look?

marcosdotme · 2024-05-20T19:18:39Z

Changed in Python 3.10. We could probably generate two sets of expected results and run that test based on different Python versions. We could try https://docs.pytest.org/en/latest/how-to/skipping.html#id1. Not sure whether there's something like parameterize we can use in this case. @marcosdotme do you want me to resolve it? or do you want to take a look?

Can you resolve this one? So I can learn from watching this time

Lee-W · 2024-05-20T19:21:07Z

Changed in Python 3.10. We could probably generate two sets of expected results and run that test based on different Python versions. We could try https://docs.pytest.org/en/latest/how-to/skipping.html#id1. Not sure whether there's something like parameterize we can use in this case. @marcosdotme do you want me to resolve it? or do you want to take a look?

Can you resolve this one? So I can learn from watching this time

Sure 🙂

…changes in python 3.10 Optional arguments -> options

Lee-W · 2024-05-20T19:59:12Z

@marcosdotme Done. I decide not to cover < 3.10 as this is not important enough to check

marcosdotme · 2024-05-20T20:46:44Z

@marcosdotme Done. I decide not to cover < 3.10 as this is not important enough to check

Ok! Thanks 🤝🏻

Lee-W · 2024-05-20T21:17:48Z

@woile @noirbizarre I'm planning on merging it these days. Please let me know if you'd like to take a look. 🙂

marcosdotme requested review from woile, Lee-W and noirbizarre as code owners May 16, 2024 19:01

github-actions bot added the pr-status: wait-for-review label May 16, 2024

This was referenced May 16, 2024

automate cli screenshot update in CI #1112

Closed

missing help text when subcommand is used (e.g., cz commit --help) #1115

Closed

woile reviewed May 18, 2024

View reviewed changes

commitizen/cli.py Show resolved Hide resolved

Lee-W reviewed May 18, 2024

View reviewed changes

Lee-W added pr-status: wait-for-modification and removed pr-status: wait-for-review labels May 18, 2024

marcosdotme requested review from Lee-W and woile May 20, 2024 01:48

github-actions bot added the pr-status: wait-for-review label May 20, 2024

Lee-W removed the pr-status: wait-for-review label May 20, 2024

marcosdotme force-pushed the issue-1115 branch from 12da1d5 to 4bb1c04 Compare May 20, 2024 18:01

github-actions bot added the pr-status: wait-for-review label May 20, 2024

marcosdotme force-pushed the issue-1115 branch 2 times, most recently from 6e92388 to 1122611 Compare May 20, 2024 18:17

marcosdotme added 3 commits May 20, 2024 15:33

fix(cli/commands): add description for subcommands

d6d0b42

refactor(tests/commands): move "other" tests for the correspondent file

6eb8704

refactor(KNOWN_SCHEMES): replace set comprehension for list comprehen…

1da181c

…sion Set is an unordered structure, so every call the order changes. Because the unordered behavior we can't use `file_regression` to test the command output.

marcosdotme force-pushed the issue-1115 branch from abd3226 to 1da181c Compare May 20, 2024 18:33

test(commands): add skip_if_below_py_3_10 as the message of argument …

bbf19ad

…changes in python 3.10 Optional arguments -> options

Lee-W approved these changes May 20, 2024

View reviewed changes

Lee-W added pr-status: ready-to-merge almost ready to merge. just keep it for a few days for others to check and removed pr-status: wait-for-review pr-status: wait-for-modification labels May 20, 2024

Lee-W merged commit 8f4dbe9 into commitizen-tools:master May 22, 2024
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add description for subcommands #1120

fix: add description for subcommands #1120

marcosdotme commented May 16, 2024

marcosdotme commented May 16, 2024 •

edited

codecov bot commented May 16, 2024 •

edited

Lee-W May 18, 2024

marcosdotme May 18, 2024

Lee-W May 18, 2024

marcosdotme May 18, 2024

marcosdotme May 19, 2024 •

edited

marcosdotme May 20, 2024

Lee-W May 20, 2024

marcosdotme May 20, 2024

marcosdotme May 20, 2024

Lee-W May 20, 2024 •

edited

marcosdotme commented May 20, 2024

Lee-W commented May 20, 2024

Lee-W commented May 20, 2024

marcosdotme commented May 20, 2024

Lee-W commented May 20, 2024

Lee-W commented May 20, 2024

Lee-W commented May 20, 2024 •

edited

marcosdotme commented May 20, 2024 •

edited

Lee-W commented May 20, 2024

Lee-W commented May 20, 2024

marcosdotme commented May 20, 2024

Lee-W commented May 20, 2024

	with open(multiple_versions_increase_string, encoding="utf-8") as f:
	file_regression.check(f.read(), extension=".txt")

fix: add description for subcommands #1120

fix: add description for subcommands #1120

Conversation

marcosdotme commented May 16, 2024

Description

Checklist

Expected behavior

Steps to Test This Pull Request

Screenshot

marcosdotme commented May 16, 2024 • edited

codecov bot commented May 16, 2024 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcosdotme May 19, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lee-W May 20, 2024 • edited

Choose a reason for hiding this comment

marcosdotme commented May 20, 2024

Lee-W commented May 20, 2024

Lee-W commented May 20, 2024

marcosdotme commented May 20, 2024

Lee-W commented May 20, 2024

Lee-W commented May 20, 2024

Lee-W commented May 20, 2024 • edited

marcosdotme commented May 20, 2024 • edited

Lee-W commented May 20, 2024

Lee-W commented May 20, 2024

marcosdotme commented May 20, 2024

Lee-W commented May 20, 2024

marcosdotme commented May 16, 2024 •

edited

codecov bot commented May 16, 2024 •

edited

marcosdotme May 19, 2024 •

edited

Lee-W May 20, 2024 •

edited

Lee-W commented May 20, 2024 •

edited

marcosdotme commented May 20, 2024 •

edited