[bazel+js]: Wrap grid-ui tests with bazel by shs96c · Pull Request #16758 · SeleniumHQ/selenium
PR Code Suggestions ✨
Explore these optional code suggestions:
| Category | Suggestion | Impact |
| General |
Add JUnit reporter
Add a reporters property to jest.config.cjs to configure jest-junit. This will
enable the generation of JUnit XML reports, making use of the newly added
dependency.
javascript/grid-ui/jest.config.cjs [30-40]
transformIgnorePatterns: [
'node_modules/(?!(\\.aspect_rules_js/.*/)?(pretty-ms|parse-ms)/)'
],
setupFilesAfterEnv: [
'<rootDir>/src/setupTests.tsx',
'<rootDir>/src/tests/setup-jest.js'
],
+reporters: [
+ 'default',
+ ['jest-junit', { outputDirectory: './reports/junit', outputName: 'js-test-results.xml' }]
+],
testEnvironmentOptions: {
suppressConsole: true
}Suggestion importance[1-10]: 7
__
Why: The PR adds the jest-junit dependency but does not configure it. This suggestion correctly proposes to add the reporter configuration in jest.config.cjs, which makes the newly added dependency functional and useful for CI environments.
| Medium
|
Use srcs for test files
In the jest_test rule, move test source files from the data attribute to the
srcs attribute. Use glob with an exclude pattern to keep non-test source files
in data.
javascript/grid-ui/BUILD.bazel [139-146]
jest_test(
name = "test",
+ srcs = glob(
+ ["src/**/*.test.ts", "src/**/*.test.tsx", "src/**/*.spec.ts", "src/**/*.spec.tsx"],
+ ),
config = "jest.config.cjs",
- data = glob(["src/**/*"]) + DEPS + TEST_DEPS + [
+ data = glob(
+ ["src/**/*"],
+ exclude = ["src/**/*.test.ts", "src/**/*.test.tsx", "src/**/*.spec.ts", "src/**/*.spec.tsx"],
+ ) + DEPS + TEST_DEPS + [
"tsconfig.json",
],
node_modules = ":node_modules",
)Suggestion importance[1-10]: 6
__
Why: The suggestion correctly points out that test files should be in the srcs attribute for jest_test for semantic correctness and to align with Bazel best practices, which can improve dependency tracking and caching.
| Low
|
|
| |