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

cli: make program outputs using the cli module testable in cli/testdata #21456

Merged
merged 4 commits into from
May 8, 2024

Conversation

ttytm
Copy link
Member

@ttytm ttytm commented May 7, 2024

Changes in regard of an issue I'm encountering with the cli module and like to fix and test cover. To make development (especially testing) more accessible, this PR adds testability of program outputs that use the cli module.

Changes per commit:

  • Makes the current test in cli_test.v namely fn test_long_description also testable as executable in vlib/cli/testdata/long_description.vv / ../long_description.out.
  • Moves the cli modue tests that currently reside in vlib/v/slow_tests/inout/ into cli/testdata (all the cli tests take usually less than 300ms)

Testing locally after checking out the PR and making a change to one of the test / out files to ensure the tests work as they should might help to simplify reviewing.

@ttytm ttytm changed the title make test current test cli_test.v also as executable to test it's o… cli: make program outputs using the cli module testable in cli/testdata May 7, 2024
Copy link
Member

@spytheman spytheman left a comment

Choose a reason for hiding this comment

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

Good work. It makes the module more self contained.

The slowdown is not too bad, even on my slow i3:
image

@spytheman
Copy link
Member

The change detection also works, and produces easily readable results:
image

@spytheman spytheman merged commit b53012a into vlang:master May 8, 2024
57 checks passed
@ttytm ttytm deleted the cli/testdata branch May 8, 2024 07:19
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.

None yet

2 participants