feat: add integration tests for React generator#224
feat: add integration tests for React generator#224vikasrao23 wants to merge 2 commits intoopen-feature:mainfrom
Conversation
Implements integration test coverage for the React generator following the standardized Dagger-based testing pattern. Changes: - test/integration/cmd/react/run.go - Dagger test runner - test/react-integration/package.json - npm configuration - test/react-integration/tsconfig.json - TypeScript config - test/react-integration/src/test.ts - Hook structure validation - test/integration/cmd/run.go - Added React test to main runner - Makefile - Added test-integration-react target The test: 1. Builds the CLI from source 2. Generates React/TypeScript hooks 3. Compiles with TypeScript 4. Validates hook API structure (useFlag, useFlagWithDetails, getKey) 5. Verifies all 5 flag types compile correctly Note: Full runtime testing of hooks requires React environment (browser/JSDOM). This test validates compilation and API structure. Acceptance criteria met: ✅ React generator covered by integration test ✅ Generated code compiles successfully ✅ Test integrated into CI suite ✅ Follows documented integration testing structure Closes open-feature#118 Signed-off-by: vikasrao23 <vikasrao23@users.noreply.github.com>
Summary of ChangesHello @vikasrao23, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the testing infrastructure by adding comprehensive integration tests for the React code generator. These tests leverage Dagger to build the CLI, generate React client code, and then validate its compilation and the structural integrity of the generated React hooks. This ensures that any changes to the React generator maintain compatibility and produce functional, well-formed code, improving the reliability of the generated SDKs. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces integration tests for the React code generator using Dagger. The changes include updates to the Makefile to add a new target for React integration tests, a new Go file (test/integration/cmd/react/run.go) to orchestrate the Dagger-based testing, and new files for the React integration test environment (package.json, tsconfig.json, src/test.ts). The run.go file in test/integration/cmd has also been updated to include the new React integration test in the overall test suite. The new tests validate the compilation of generated React code, the existence of hook functions with correct signatures, and the functionality of the .getKey() method. Overall, the changes are well-structured and add valuable testing coverage.
| WithExec([]string{"apk", "add", "--no-cache", "git"}). | ||
| WithDirectory("/src", source). | ||
| WithWorkdir("/src"). | ||
| WithExec([]string{"go", "mod", "tidy"}). | ||
| WithExec([]string{"go", "mod", "download"}). | ||
| WithExec([]string{"go", "build", "-o", "cli", "./cmd/openfeature"}) |
There was a problem hiding this comment.
It's good practice to specify the exact version of golangci-lint or go tools in Dagger containers to ensure reproducible builds and avoid unexpected changes due to upstream updates. While golang:1.24-alpine is specific, the go mod tidy and go mod download commands could potentially pull different versions of dependencies over time. Consider pinning specific versions for critical tools or dependencies.
There was a problem hiding this comment.
Good point! I've pinned the Go version to golang:1.24.3-alpine for reproducible builds. This ensures the exact same Go toolchain is used across all test runs. ✅
test/integration/cmd/react/run.go
Outdated
| From("node:20-alpine"). | ||
| WithWorkdir("/app"). | ||
| WithDirectory("/app", testFiles). | ||
| WithDirectory("/app/src/generated", generatedFiles). | ||
| WithExec([]string{"npm", "install"}). | ||
| WithExec([]string{"npm", "run", "build"}). | ||
| WithExec([]string{"node", "dist/test.js"}) |
There was a problem hiding this comment.
Similar to the Go build environment, consider pinning specific versions for node and npm dependencies in the Dagger container to ensure consistent test results across different runs and environments. For example, instead of node:20-alpine, you might use node:20.x.y-alpine if a specific patch version is desired, and explicitly define npm versions in package.json.
There was a problem hiding this comment.
Absolutely! I've pinned the Node version to node:20.18.1-alpine to ensure consistent npm and Node.js behavior across test runs. This prevents unexpected changes from upstream updates. ✅
test/react-integration/package.json
Outdated
| "@openfeature/react-sdk": "^1.0.0", | ||
| "@openfeature/server-sdk": "^1.34.0", | ||
| "react": "^18.2.0" |
There was a problem hiding this comment.
It's generally recommended to use exact versions for dependencies in package.json for integration tests to ensure reproducibility. Using ^1.0.0 or ^1.34.0 allows for minor and patch updates, which could potentially introduce breaking changes or unexpected behavior in the test environment. Consider using exact versions like "@openfeature/react-sdk": "1.0.0".
| "@openfeature/react-sdk": "^1.0.0", | |
| "@openfeature/server-sdk": "^1.34.0", | |
| "react": "^18.2.0" | |
| "@openfeature/react-sdk": "1.0.0", | |
| "@openfeature/server-sdk": "1.34.0", | |
| "react": "18.2.0" |
There was a problem hiding this comment.
Great suggestion! I've switched to exact versions for all dependencies:
@openfeature/react-sdk:1.0.2@openfeature/server-sdk:1.34.0react:18.2.0
This prevents unexpected breaking changes or behavior differences in the test environment. ✅
test/react-integration/package.json
Outdated
| "@types/node": "^20.0.0", | ||
| "@types/react": "^18.2.0", | ||
| "typescript": "^5.3.0" |
There was a problem hiding this comment.
Similar to direct dependencies, using exact versions for devDependencies in package.json helps maintain a stable and reproducible test environment. Consider pinning these to exact versions.
| "@types/node": "^20.0.0", | |
| "@types/react": "^18.2.0", | |
| "typescript": "^5.3.0" | |
| "@types/node": "20.0.0", | |
| "@types/react": "18.2.0", | |
| "typescript": "5.3.0" |
There was a problem hiding this comment.
Agreed! I've pinned all devDependencies to exact versions:
@types/node:20.17.10@types/react:18.2.79typescript:5.3.3
This maintains a stable and reproducible test environment across all runs. ✅
test/react-integration/src/test.ts
Outdated
| console.log('All generated React hooks are properly structured!'); | ||
| console.log('Generated React code compiles successfully!'); | ||
| process.exit(0); | ||
| } catch (error: any) { |
There was a problem hiding this comment.
The error: any type annotation is too broad. It's better to use a more specific type if possible, or at least unknown and then narrow it down. This improves type safety and makes the code more robust.
| } catch (error: any) { | |
| } catch (error: unknown) { |
There was a problem hiding this comment.
Excellent point! I've changed it to error: unknown and added proper type narrowing:
} catch (error: unknown) {
const message = error instanceof Error ? error.message : String(error);
console.error('Error:', message);
process.exit(1);
}This improves type safety and makes the code more robust. ✅
- Pin exact Go version to golang:1.24.3-alpine for reproducible builds - Pin exact Node version to node:20.18.1-alpine for consistency - Use exact dependency versions in package.json to prevent unexpected updates - Replace 'error: any' with 'error: unknown' for better type safety - Add proper error message handling with type narrowing All changes improve test reproducibility and code robustness. Refs: open-feature#118 Signed-off-by: Vikas Rao <mvrao@uci.edu>
2 participants
Fixes #118
Adds Dagger-based integration tests for the React code generator.
Changes
test/integration/cmd/react/run.go- Dagger test runnertest/react-integration/package.json- npm project with @openfeature/react-sdktest/react-integration/tsconfig.json- TypeScript configurationtest/react-integration/src/test.ts- Hook structure validationMakefile- Addedtest-integration-reacttargettest/integration/cmd/run.go- Integrated into main runnerTest Coverage
✅ CLI builds from source
✅ React/TypeScript code generation
✅ TypeScript compilation
✅ Hook API structure validation (
useFlag,useFlagWithDetails,getKey)✅ All 5 flag types validated
Note on Testing Approach
React hooks require a React environment (component tree, context providers) to execute. This test validates:
.getKey()method works (doesn't need React context)Full runtime testing would require browser/JSDOM environment.
Run It
Signed-off-by: vikasrao23 vikasrao23@users.noreply.github.com