-
Notifications
You must be signed in to change notification settings - Fork 1k
Enhancements: CLI Test Cases #144
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
Enhancements: CLI Test Cases #144
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for your work!

I found those tests quite slow to run when i tried them in a codespace (4 cores 8gb ram)
Being slow to run means they risk being less helpful in a TDD context and I don't think that's something we want from the main test suite
So we can see it one of two ways:
1 - They are end-to-end tests and should be separated from the main test suite
2 - They are unit test so they should mock the necessary components to only test units and not integration
Happy to discuss this further if it's unclear what I mean by that
|
@cyclotruc Made a change to path which reduced the execution time right now, Initally had the path outside of directory, which should have been "./"
|
cyclotruc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!
I tested it by breaking the CLi on purpose and the test failed, which is all we really needed
Co-authored-by: Gowtham Kishore <70622086+gowthamkishore3799@users.noreply.github.com>

Summary:
Implementation Notes:
Alternative Options Considered:
Instead of actually creating and removing the output file during the test, I considered mocking the os.open and related functions. However, I opted to test the entire process end-to-end to ensure seamless functionality.