Skip to content

Conversation

DevelopmentCats
Copy link
Contributor

Closes #383

Description

  • Update CONTRIBUTION.md to elaborate on ts and tf tests
  • Add ./scripts/terraform_test_all.sh to CI for ts tests

Type of Change

  • New module
  • Bug fix
  • Feature/enhancement
  • Documentation
  • Other

Testing & Validation

  • Tests pass (bun test)
  • Code formatted (bun run fmt)
  • Changes tested locally

@Copilot Copilot AI review requested due to automatic review settings August 25, 2025 16:03
Copy link
Contributor

@Copilot Copilot AI left a 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.

CONTRIBUTING.md Outdated
@@ -124,18 +124,27 @@ This script generates:
- Accurate description and usage examples
- Correct icon path (usually `../../../../.icons/your-icon.svg`)
- Proper tags that describe your module
3. **Create at least one `.tftest.hcl`** to test your module with `terraform test`
3. **Create tests for your module:**
- **New modules**: Create `.tftest.hcl` files and test with `terraform test`
Copy link
Member

@matifali matifali Aug 25, 2025

Choose a reason for hiding this comment

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

Let's also include TypeScript testing for new modules, as we can not test the business logic of scripts with Terraform-only tests.

Copy link
Member

@matifali matifali left a 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.

@DevelopmentCats
Copy link
Contributor Author

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

Copy link
Member

@matifali matifali left a 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

matifali
matifali previously approved these changes Aug 26, 2025
@DevelopmentCats
Copy link
Contributor Author

Can we update the script in package.json to run all tests when someone run bun 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"
  },

@DevelopmentCats
Copy link
Contributor Author

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.

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.

Update Contribution Docs for TS and TF tests
2 participants