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

[WIP] - Add tests for qse init command with docker-compose flag #106

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

afmireski
Copy link

@afmireski afmireski commented Feb 6, 2025

Resolves #98

Description

Tests were implemented to verify the behavior of the --docker-compose flag in the init command.

Based on the discussions in the issue and the challenges faced by @alguiguilo098, I followed the suggestions and introduced a new flag: --add-cache-service.

Added the --no-db flag to allow bypassing the needDb prompt check in templates that had this option enabled.

  • All new an existing tests passed

Waiting @Ashrockzzz2003 review.

@afmireski
Copy link
Author

@Ashrockzzz2003 , I skipped the template tests that have the needDb: true setting because this config prompts the user to confirm whether they want to add the database images. As a result, these tests would hang waiting for user input until they time out.

Would you find it appropriate to replace the needDb config with a --no-db flag, similar to the cache service? Given that the default behavior is to add the database image.

https://github.com/afmireski/quick_start_express/blob/07075a46ddb2c6ced913efa0d19b3b1888dfb0e8/bin/util/docker.js#L5-L16

Copy link
Member

@Ashrockzzz2003 Ashrockzzz2003 left a comment

Choose a reason for hiding this comment

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

@afmireski
Thanks for moving the files to ignore while computing sha256 to a list from if. Codebase looks better there now.

About the needDb flag, yes, you can add a flag.

I see you've removed the code for the user prompt. Instead, comment it out. In future when we find a way to test it, we might need it. Please do comment out that code instead of removing it. Same for the new flag needDb

Overall looks good!

At last please run npm run format and then check if npm run test:format exits without an error. We're following standard code formatting across the codebase to ensure they are not highlighted as a change in PRs. This check is part of the CI too!

Also, add yourselves to contributors in README.

@afmireski
Copy link
Author

Hello, @Ashrockzzz2003. I've made the suggested changes and implemented the --no-db flag.

I considered mocking either the confirm or userPrompt function in the tests using Jest, but I wasn't successful. So, I kept the new flag implementation. At least in my tests, all cases passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request testing
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Add tests for qse init command with docker-compose flag.
2 participants