-
Notifications
You must be signed in to change notification settings - Fork 59
chore: update CONTRIBUTION docs to explain both tests, and update CI for both tests #384
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
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.
Pull Request Overview
This PR updates the documentation and CI pipeline to clarify that the repository supports both TypeScript and Terraform testing approaches. The changes align the documentation with the actual testing practices and ensure both test types run in CI.
- Update CONTRIBUTING.md to explain both TypeScript tests (
bun test) and Terraform tests (terraform test) - Add Terraform test execution to the CI workflow
- Clarify testing requirements for new vs existing modules
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| CONTRIBUTING.md | Updated documentation to explain dual testing approaches and provide clear guidance for both TypeScript and Terraform tests |
| .github/workflows/ci.yaml | Added Terraform test execution step to ensure both test types run in CI |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Atif Ali <atif@coder.com>
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.
left a few suggestions. I think we need to have both simultaneously.
TypeScript tests can be skipped if the module does not run any scripts.
left a few suggestions. I think we need to have both simultaneously.
TypeScript tests can be skipped if the module does not run any scripts.
Ahh I okay I'm seeing the whole picture better now. I'll commit your other change and give it one more look over before it's merged
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.
Can we update the script in package.json to run all tests when someone run bun test
Can we update the script in
package.jsonto run all tests when someone runbun test
Doesn't the package.json currently do this already?
"name": "registry",
"scripts": {
"fmt": "bun x prettier --write . && terraform fmt -recursive -diff",
"fmt:ci": "bun x prettier --check . && terraform fmt -check -recursive -diff",
"terraform-validate": "./scripts/terraform_validate.sh",
"test": "./scripts/terraform_test_all.sh",
"update-version": "./update-version.sh"
},
I think we just need to omit the examples from the terraform_test_all.sh script. I will do this tomorrow morning and get this finished up.
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.
Current changes look good! Just wasn't sure if there was some extra context we could add
Closes #383
Description
Type of Change
Testing & Validation
bun test)bun run fmt)