Skip to content

Conversation

@Fellmonkey
Copy link
Contributor

@Fellmonkey Fellmonkey commented Nov 4, 2025

Introduces a 'Run Tests' job to the build validation workflow for multiple SDKs. The step conditionally runs tests for each SDK based on the presence of test files or configuration, providing feedback if no tests are found or configured.

What does this PR do?

This PR adds a "Run Tests" step to the sdk-build-validation.yml GitHub Actions workflow. The new step implements conditional test execution for various SDKs (web, node, cli, react-native, flutter, apple, swift, android, kotlin, php, python, ruby, dart, go, dotnet) generated during the CI build process.

For each SDK, the code checks for the presence of test files or configurations and runs the appropriate test commands:

  • Node.js-based SDKs (web, node, cli, react-native): Runs npm test if a valid test script exists in package.json
  • Flutter: Runs flutter test if Dart test files are found in the test directory
  • Apple/Swift: Runs swift test if Swift test files are found in the Tests directory
  • Android: Runs ./gradlew test if test directories exist
  • Kotlin: Runs ./gradlew test if src/test directory exists
  • PHP: Runs vendor/bin/phpunit tests/ if PHPUnit is configured
  • Python: Runs python -m pytest if test files are found
  • Ruby: Runs bundle exec rake test or bundle exec rspec based on configuration
  • Dart: Runs dart test if test files are found
  • Go: Runs go test Documents. if Go test files are found
  • .NET: Runs dotnet test if test SDK is configured in project files

If no tests are found for a particular SDK, the step outputs an appropriate message and continues without failing the build.

Test Plan

Push this change to a branch and create a PR to trigger the workflow. Verify that the "Run Tests" step executes for all matrix combinations and produces expected output (either test results or "No tests found" messages).
action: https://github.com/Fellmonkey/sdk-generator/actions/runs/19081542717/job/54511219611

Related PRs and Issues

#1221 - used a prototype for automatic test execution with CI

Have you read the Contributing Guidelines on issues?

Yes

Summary by CodeRabbit

  • Chores
    • CI workflow now automatically runs tests across all supported SDKs using language-appropriate test commands and runners.
    • Enhanced build pipeline to include test execution for web, mobile, backend, and other development environments.
    • Intelligently skips test execution and displays informative messages when tests are not configured for a specific project.

Introduces a 'Run Tests' job to the build validation workflow for multiple SDKs. The step conditionally runs tests for each SDK based on the presence of test files or configuration, providing feedback if no tests are found or configured.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025

Walkthrough

The change adds a new "Run Tests" step to the SDK build validation CI workflow that executes language-specific test commands for each SDK entry in the build matrix. The step conditionally runs tests based on SDK type: npm test for web/node/cli/react-native SDKs with placeholder-test fallback handling, and respective test invocations for flutter, swift, android, kotlin, php, python, ruby, dart, go, and dotnet. The implementation includes detection logic to skip test execution when tests are not configured for a particular SDK, emitting informative messages instead.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify each SDK type has correct test command syntax
  • Review conditional logic for test detection and fallback behavior (particularly placeholder-test handling for npm-based SDKs)
  • Confirm the step placement within the workflow doesn't interfere with existing build steps
  • Validate matrix variable interpolation for different language runtimes

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and concisely describes the main change: adding a test execution step to the SDK build workflow.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
.github/workflows/sdk-build-validation.yml (2)

252-258: Improve clarity of find command precedence in Python test detection.

The find . -maxdepth 2 -name "test_*.py" -o -name "*_test.py" command is functionally correct, but the operator precedence could be clearer with explicit grouping. While find applies -maxdepth globally, parentheses would make this intent obvious to future maintainers.

  python)
-   if [ -d "tests" ] || find . -maxdepth 2 -name "test_*.py" -o -name "*_test.py" 2>/dev/null | grep -q .; then
+   if [ -d "tests" ] || find . -maxdepth 2 \( -name "test_*.py" -o -name "*_test.py" \) 2>/dev/null | grep -q .; then
      python -m pytest
    else
      echo "No pytest tests found"
    fi

231-237: Android test detection assumes project-specific directory structure.

The checks for library/src/test and app/src/test are tailored to the Appwrite Android SDK's layout. This works for generated SDKs following this structure but won't detect tests in other directory arrangements. As long as the SDK generator produces this structure consistently, this is acceptable; document this assumption if it's not obvious from context.

Consider adding a comment in the workflow to document why these specific paths are checked, making it clearer for future maintainers why standard Gradle paths like src/test (used for Kotlin) weren't chosen here.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd71267 and 090c572.

📒 Files selected for processing (1)
  • .github/workflows/sdk-build-validation.yml (1 hunks)
🔇 Additional comments (2)
.github/workflows/sdk-build-validation.yml (2)

201-296: Solid test execution strategy with appropriate detection logic for each language.

The step structure mirrors the build logic above and uses sound heuristics to detect and run tests per SDK. Test failures will correctly cause workflow failures, while non-existent tests are gracefully skipped with informative messages.


205-216: Node.js placeholder detection is a reasonable heuristic with known limitations.

The grep pattern '"test".*"echo.*no test' targets generated SDKs with placeholder test scripts. Be aware this will not detect all placeholder formats (e.g., if a generated SDK uses a different error message structure). If you observe false negatives (placeholder tests running) during CI runs, you may need to expand this pattern or adjust the SDK generator to use a consistent placeholder format.

Can you confirm whether the generated SDKs consistently use "echo.*no test" placeholders, or whether additional patterns should be detected?

@Fellmonkey Fellmonkey mentioned this pull request Nov 4, 2025
@Fellmonkey
Copy link
Contributor Author

There is also a problem with php.
Your discretion on how to fix it.
image

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.

1 participant