feat: add e2e and unit test #307
Conversation
WalkthroughThe changes in this pull request involve updates to multiple files related to the configuration and structure of an end-to-end testing environment for the Galacean App. Key modifications include adjustments to the Changes
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Outside diff range and nitpick comments (12)
vitest.config.ts (2)
3-10: LGTM: Vitest configuration looks good, with room for enhancementThe configuration is correctly structured and exports a default configuration object using
defineConfig. The test coverage options are set up properly with Istanbul as the provider and HTML reporter.Consider the following enhancements:
- Add more reporters for different output formats, e.g., 'lcov' for CI integration:
reporter: ['html', 'lcov', 'text']- Include other common Vitest options like
globals,environment, orincludeif needed for your project.Would you like me to provide an example of a more comprehensive Vitest configuration?
1-10: Overall: Good start, but consider expanding for e2e and unit testsThe Vitest configuration provides a solid foundation for test coverage. However, given that the PR objectives mention adding both e2e and unit tests, consider expanding this configuration to explicitly support both types of tests. This could involve:
- Setting up separate configurations or environments for e2e and unit tests.
- Configuring different coverage thresholds for e2e and unit tests.
- Adding specific include/exclude patterns for different test types.
Would you like assistance in setting up a more comprehensive configuration that addresses both e2e and unit testing scenarios?
tests/vitest.config.ts (1)
14-20: LGTM: Browser testing configuration looks good.The setup for browser testing using Playwright with Chromium is appropriate for end-to-end testing. This configuration will allow for robust testing in a real browser environment.
Consider expanding the browser testing to include other major browsers (e.g., Firefox, Safari) in the future to ensure cross-browser compatibility. This can be achieved by adding more browser configurations to the test setup.
e2e/package.json (3)
6-11: Scripts are well-defined, but consider enhancing the e2e script.The scripts cover the essential development and testing needs using Vite, which is a good choice. However, the e2e script might benefit from additional configuration.
Consider updating the e2e script to include more options or use a dedicated test runner for end-to-end tests. For example:
- "e2e": "vite-node test.ts" + "e2e": "vitest run -c ./vitest.config.ts"This assumes you'll create a
vitest.config.tsfile to configure your end-to-end tests, which can provide more flexibility and features specific to e2e testing.
12-16: DevDependencies are good, but consider adding testing frameworks.The current devDependencies are appropriate for a TypeScript-based Vite project. However, for a comprehensive e2e testing setup, you might want to consider adding more testing-related dependencies.
Consider adding the following devDependencies for a more robust testing environment:
"devDependencies": { "ts-node": "^10", "vite": "^5", - "vite-node": "^2.0.5" + "vite-node": "^2.0.5", + "vitest": "^0.34.0", + "@playwright/test": "^1.38.0", + "@types/node": "^20.5.9" },These additions will provide:
- Vitest for unit and integration testing
- Playwright for browser-based end-to-end testing
- Type definitions for Node.js
17-21: Dependencies look good, but consider version consistency.The dependencies include the necessary Galacean engine and toolkit libraries. The use of a workspace reference for @galacean/engine-toolkit is appropriate for local development.
For better version consistency and to avoid potential conflicts, consider aligning the versions of @galacean/engine and @galacean/engine-physics-physx with the version of @galacean/engine-toolkit. If they're meant to be used together, they should ideally have matching major and minor versions.
"dependencies": { - "@galacean/engine": "^1.3.0", - "@galacean/engine-physics-physx": "^1.3.0", + "@galacean/engine": "^1.3.0-beta.0", + "@galacean/engine-physics-physx": "^1.3.0-beta.0", "@galacean/engine-toolkit": "workspace:*" }This assumes that the workspace version of @galacean/engine-toolkit is at 1.3.0-beta.0. Adjust accordingly based on the actual version in your workspace.
tests/src/OrbitControl.test.ts (2)
11-11: LGTM: Setup function updated to use beforeAllThe
beforefunction has been correctly replaced withbeforeAll, which is the appropriate Vitest equivalent. This change maintains the intended functionality of running the setup once before all tests in the suite.Consider adding a cleanup step using
afterAllto dispose of the engine and any resources created during the test. This ensures proper cleanup after the tests and can prevent potential memory leaks or interference with other tests. For example:afterAll(() => { engine.destroy(); });
Line range hint
17-20: Suggestion: Enhance test coverageWhile the existing test case for the constructor is valid, consider adding more test cases to improve coverage of the OrbitControl component. Some suggestions:
- Test the functionality of enabling/disabling autoRotate.
- Verify that changing autoRotateSpeed affects the rotation speed.
- Test any public methods or properties of the OrbitControl component.
- Add tests for edge cases or error handling if applicable.
Expanding the test suite will help ensure the component behaves correctly under various scenarios.
packages/controls/package.json (1)
22-22: LGTM! Consider adding a comment for clarity.The addition of the
debugfield pointing tosrc/index.tsis a good practice for debugging purposes. It can help developers quickly locate the main source file.Consider adding a brief comment in the
package.jsonfile to explain the purpose of this non-standard field. For example:+ // Custom field for debugging: points to the main source file "debug": "src/index.ts",This will help other developers understand the intention behind this field.
package.json (2)
14-15: LGTM: Updated test scripts with VitestThe switch to Vitest for testing and the addition of a coverage script with UI are good improvements. These changes simplify the testing process and add new capabilities.
Consider updating the
ciscript on line 16 to use the newvitestcommand instead of the oldtest-covscript, which has been removed according to the AI summary.
Line range hint
1-78: Summary of changes and their impactThe changes to
package.jsonsignificantly improve the project's testing infrastructure by:
- Introducing Vitest for unit testing and coverage.
- Adding e2e testing capabilities with Playwright.
- Updating TypeScript to version 5.
These changes align well with the PR objectives and the AI-generated summary. They should lead to a more robust and efficient testing process.
Consider the following points:
- Ensure all team members are familiar with Vitest and Playwright for smooth adoption.
- Update any CI/CD pipelines to use the new testing commands.
- Review and update any documentation related to the testing process.
- Plan for any necessary refactoring of existing tests to work with the new testing framework.
e2e/cases/index.ts (1)
5-7: Consider removing console log statements or replacing them with proper logging mechanismsThe
console.logstatements in the event listeners might clutter the console output in production. If these logs are for debugging purposes, consider removing them or using a dedicated logging library that can be configured for different environments.Also applies to: 9-11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (16)
- .gitignore (2 hunks)
- e2e/.gitignore (1 hunks)
- e2e/cases/index.ts (1 hunks)
- e2e/index.html (1 hunks)
- e2e/package.json (1 hunks)
- e2e/test.ts (1 hunks)
- e2e/tests/basic.test.ts (1 hunks)
- e2e/vite.config.ts (1 hunks)
- package.json (2 hunks)
- packages/controls/package.json (1 hunks)
- pnpm-workspace.yaml (1 hunks)
- tests/index.ts (0 hunks)
- tests/src/OrbitControl.test.ts (1 hunks)
- tests/vitest.config.ts (1 hunks)
- vitest.config.ts (1 hunks)
- vitest.workspace.ts (1 hunks)
💤 Files with no reviewable changes (1)
- tests/index.ts
✅ Files skipped from review due to trivial changes (3)
- e2e/.gitignore
- e2e/index.html
- vitest.workspace.ts
🧰 Additional context used
🔇 Additional comments (14)
pnpm-workspace.yaml (1)
6-6: LGTM! Verify the 'e2e' directory setup.The addition of 'e2e' to the workspace packages list is appropriate for including end-to-end tests in the project structure. This change aligns well with the PR objectives of adding e2e tests.
To ensure the 'e2e' directory is properly set up, please run the following verification script:
This script will help confirm that the 'e2e' directory is properly set up with the necessary files for end-to-end testing.
✅ Verification successful
LGTM! The 'e2e' directory is properly set up with necessary files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and basic structure of the 'e2e' directory # Check if the 'e2e' directory exists if [ -d "e2e" ]; then echo "The 'e2e' directory exists." # List contents of the 'e2e' directory echo "Contents of the 'e2e' directory:" ls -la e2e # Check for common test-related files if [ -f "e2e/package.json" ]; then echo "package.json found in 'e2e' directory." else echo "Warning: package.json not found in 'e2e' directory." fi # Check for test files TEST_FILES=$(find e2e -name "*test*.ts" -o -name "*spec*.ts") if [ -n "$TEST_FILES" ]; then echo "Test files found in 'e2e' directory:" echo "$TEST_FILES" else echo "Warning: No test files (*.test.ts or *.spec.ts) found in 'e2e' directory." fi else echo "Warning: The 'e2e' directory does not exist." fiLength of output: 1151
vitest.config.ts (1)
1-1: LGTM: Correct import statementThe import statement correctly imports
defineConfigfrom "vitest/config", which is the recommended way to define Vitest configurations with type checking..gitignore (3)
6-6: Approved: Expanded scope for ignoringnode_modulesThe modification from
/node_modulestonode_modulesexpands the scope of what's being ignored. This change ensures that allnode_modulesdirectories throughout the repository are ignored, not just the one at the root level. This is a common and recommended practice in Git repositories.
28-28: Approved: Ignoring Playwright test reportsThe addition of
playwright-reportto the.gitignorefile is appropriate. This directory typically contains test reports generated by the Playwright end-to-end testing framework. Ignoring these reports is a good practice as they are regenerated during each test run and don't need to be version controlled. This change aligns well with the PR objective of adding e2e tests.
29-29: Approved: Ignoring test results directoryThe addition of
test-resultsto the.gitignorefile is appropriate. This directory is likely used to store various test artifacts and results. Ignoring these files is a good practice as they are typically regenerated during each test run and don't need to be version controlled. This change aligns well with the PR objective of adding tests.tests/vitest.config.ts (3)
2-2: LGTM: Import statement is correct.The import of
defineProjectfrom "vitest/config" is appropriate for setting up the Vitest configuration.
1-21: Overall, the Vitest configuration looks well-structured and appropriate.The configuration file sets up the Vitest project with specific optimizations and browser testing using Playwright. The structure and syntax are correct, and it provides a solid foundation for testing.
Key points:
- The import statement is correct.
- Package exclusions from optimization need verification.
- Browser testing setup with Playwright and Chromium is appropriate.
Consider the suggestions made in previous comments to ensure the configuration is optimal for your project's needs.
5-13: Verify the necessity of excluding @galacean packages from optimization.The configuration excludes several @galacean packages from optimization. While this might be intentional, it's important to confirm that this exclusion is necessary and aligns with the project's requirements.
Could you please explain the reasoning behind excluding these specific packages from optimization? This will help ensure that the configuration is optimal for the project's needs.
To verify the impact of this exclusion, you can run the following script:
This script will help identify where these packages are used in the project, which can provide context for their exclusion from optimization.
e2e/package.json (1)
1-5: LGTM: Basic package information is well-defined.The package information is correctly set up for an internal end-to-end testing environment. The use of a scoped package name, private flag, and module type are all appropriate choices.
tests/src/OrbitControl.test.ts (1)
3-3: LGTM: Import statement updated to use VitestThe import statement has been correctly updated to use Vitest instead of Chai, which aligns with the PR objective of updating the testing framework. The necessary testing functions (describe, beforeAll, it, expect) are now imported and used consistently throughout the test suite.
package.json (4)
6-7: LGTM: Addition of e2e workspaceThe addition of "e2e/*" to the workspaces array is appropriate for incorporating end-to-end testing into the project structure. This change aligns well with the PR objectives.
22-22: LGTM: Addition of e2e scriptThe new e2e script is a good addition that complements the new e2e workspace. It provides a convenient way to run end-to-end tests, which aligns with the PR objectives.
38-40: LGTM: Addition of Vitest and Playwright dependenciesThe addition of Vitest-related dependencies (@vitest/browser, @vitest/coverage-istanbul, @vitest/ui) and Playwright aligns well with the switch to Vitest for testing and the introduction of e2e testing capabilities. These are solid choices for improving the project's testing infrastructure.
Also applies to: 48-48
56-57: LGTM with suggestion: Updated TypeScript and added VitestThe update to TypeScript version 5 and the addition of Vitest as a dev dependency are good improvements. However, the major version bump for TypeScript might introduce breaking changes.
Please verify that all parts of the project are compatible with TypeScript 5. You can run the following command to check for any type-related issues:
Also, ensure that the removal of the other dependencies (chai, mocha, electron, floss, nyc, ts-node) mentioned in the AI summary doesn't break any existing functionality.
| import { Page } from "playwright"; | ||
|
|
||
| export function run(page: Page) { | ||
| page.mouse.move(250, 265); | ||
| page.mouse.down(); | ||
| page.mouse.move(250, 400); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing the test structure and robustness
While the current implementation provides a basic framework for simulating mouse actions, there are several areas where it could be improved:
-
The function lacks assertions, which are crucial for validating the expected behavior in a test scenario. Consider adding assertions to verify the outcome of the mouse actions.
-
The use of hardcoded coordinates (250, 265) and (250, 400) may make the test brittle and dependent on specific screen sizes or layouts. It's recommended to use more robust selectors or relative positioning.
-
The mouse is left in a pressed state after the actions, which could potentially affect subsequent tests. Consider adding a
page.mouse.up()action at the end of the function. -
The purpose of this test is not immediately clear from the implementation. Consider adding comments or renaming the function to better describe its intent.
Here's a suggested refactor to address these points:
import { Page, expect } from "playwright";
/**
* Simulates a vertical drag action and verifies its effect
* @param page The Playwright Page object
*/
export async function testVerticalDrag(page: Page) {
// Use more robust selectors instead of hardcoded coordinates
const dragHandle = await page.locator('.drag-handle');
const dragTarget = await page.locator('.drop-target');
// Get the bounding boxes of the elements
const handleBox = await dragHandle.boundingBox();
const targetBox = await dragTarget.boundingBox();
if (!handleBox || !targetBox) {
throw new Error("Could not locate drag elements");
}
// Perform the drag action
await page.mouse.move(handleBox.x + handleBox.width / 2, handleBox.y + handleBox.height / 2);
await page.mouse.down();
await page.mouse.move(targetBox.x + targetBox.width / 2, targetBox.y + targetBox.height / 2);
await page.mouse.up();
// Add an assertion to verify the drag action
const result = await page.locator('.drag-result').textContent();
expect(result).toBe("Drag successful");
}This refactored version addresses the issues by using more robust selectors, completing the mouse action cycle, and including an assertion. It also provides a clearer purpose through naming and comments.
| @@ -0,0 +1,7 @@ | |||
| import { Page } from "playwright"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider importing additional Playwright utilities
The current import statement only brings in the Page type from Playwright. Given that this is a test file, it might be beneficial to import additional utilities that are commonly used in Playwright tests.
Consider updating the import statement as follows:
import { Page, expect, test } from "@playwright/test";This change would allow you to use Playwright's test function for structuring your tests and the expect function for assertions, providing a more complete testing framework.
| const page = await browser.newPage(); | ||
| page.setViewportSize({ width: 500, height: 500 }); | ||
|
|
||
| const pages = fs.readdirSync(path.join(__dirname, "pages")); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use asynchronous file operations instead of synchronous ones
Using synchronous file system methods like fs.readdirSync and fs.existsSync can block the event loop and degrade performance. It's better to use their asynchronous counterparts to improve efficiency.
Apply this diff to refactor the code:
- const pages = fs.readdirSync(path.join(__dirname, "pages"));
+ const pages = await fs.promises.readdir(path.join(__dirname, "pages"));
...
- if (fs.existsSync(path.join(__dirname, "tests", `${pageName}.ts`))) {
+ if (
+ await fs.promises
+ .access(path.join(__dirname, "tests", `${pageName}.test`))
+ .then(() => true)
+ .catch(() => false)
+ ) {Also applies to: 24-24
| async function runTests() { | ||
| const server = await createServer({ | ||
| root: path.join(process.cwd()) | ||
| }); | ||
| await server.listen(); | ||
| server.printUrls(); | ||
|
|
||
| const page = await browser.newPage(); | ||
| page.setViewportSize({ width: 500, height: 500 }); | ||
|
|
||
| const pages = fs.readdirSync(path.join(__dirname, "pages")); | ||
| for (let i = 0; i < pages.length; i++) { | ||
| const pageName = path.basename(pages[i], ".html"); | ||
| await page.goto(`http://localhost:1237/pages/${pageName}.html`); | ||
| if (fs.existsSync(path.join(__dirname, "tests", `${pageName}.ts`))) { | ||
| const { run } = await import(`./tests/${pageName}.test`); | ||
| if (run) run(page); | ||
| } | ||
| } | ||
|
|
||
| browser.close() | ||
| server.close() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and ensure resource cleanup
To handle potential errors during the test execution and to ensure that the browser and server are properly closed even if an error occurs, consider wrapping your test logic in a try...catch...finally block. This approach enhances the reliability and maintainability of your test script.
Here's how you can refactor the code:
async function runTests() {
const server = await createServer({
root: path.join(process.cwd()),
});
await server.listen();
const address = server.httpServer.address();
const port = typeof address === "string" ? address : address.port;
server.printUrls();
const browser = await playwright.chromium.launch({
headless: false,
});
try {
const page = await browser.newPage();
page.setViewportSize({ width: 500, height: 500 });
const pages = await fs.promises.readdir(path.join(__dirname, "pages"));
for (let i = 0; i < pages.length; i++) {
const pageName = path.basename(pages[i], ".html");
await page.goto(`http://localhost:${port}/pages/${pageName}.html`);
if (
await fs.promises
.access(path.join(__dirname, "tests", `${pageName}.test`))
.then(() => true)
.catch(() => false)
) {
const { run } = await import(`./tests/${pageName}.test`);
if (run) await run(page);
}
}
} catch (error) {
console.error("Error during test execution:", error);
} finally {
await browser.close();
await server.close();
}
}| browser.close() | ||
| server.close() |
There was a problem hiding this comment.
Add 'await' when closing browser and server
Playwright's browser.close() and Vite's server.close() are asynchronous functions that return promises. To ensure that the browser and server are properly closed before the script exits, you should await these calls.
Apply this diff to fix the issue:
+ await browser.close();
+ await server.close();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| browser.close() | |
| server.close() | |
| await browser.close(); | |
| await server.close(); |
| const pages = fs.readdirSync(path.join(__dirname, "pages")); | ||
| for (let i = 0; i < pages.length; i++) { | ||
| const pageName = path.basename(pages[i], ".html"); | ||
| await page.goto(`http://localhost:1237/pages/${pageName}.html`); |
There was a problem hiding this comment.
Retrieve server port dynamically instead of hardcoding
Hardcoding the port number (1237) may cause issues if the server runs on a different port, especially in different environments or configurations. It's better to retrieve the actual port number from the server instance after it starts listening.
Apply this diff to make the port dynamic:
+ const address = server.httpServer.address();
+ const port = typeof address === 'string' ? address : address.port;
...Committable suggestion was skipped due to low confidence.
| const pos = cameraEntity.transform.position; | ||
| pos.set(10, 10, 10); | ||
| cameraEntity.transform.position = pos; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify position setting for cameraEntity
You can directly set the position without creating an extra variable or redundant assignment. This makes the code cleaner.
Apply this diff to simplify the code:
-const pos = cameraEntity.transform.position;
-pos.set(10, 10, 10);
-cameraEntity.transform.position = pos;
+cameraEntity.transform.position.set(10, 10, 10);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const pos = cameraEntity.transform.position; | |
| pos.set(10, 10, 10); | |
| cameraEntity.transform.position = pos; | |
| cameraEntity.transform.position.set(10, 10, 10); |
| const color = mtl.baseColor; | ||
| color.r = 0.0; | ||
| color.g = 0.8; | ||
| color.b = 0.5; | ||
| color.a = 1.0; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify color assignment by using the set method
Instead of setting each color component individually, you can use the set method to assign all values at once, making the code more concise.
Apply this diff to simplify the color assignment:
const color = mtl.baseColor;
-color.r = 0.0;
-color.g = 0.8;
-color.b = 0.5;
-color.a = 1.0;
+color.set(0.0, 0.8, 0.5, 1.0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const color = mtl.baseColor; | |
| color.r = 0.0; | |
| color.g = 0.8; | |
| color.b = 0.5; | |
| color.a = 1.0; | |
| const color = mtl.baseColor; | |
| color.set(0.0, 0.8, 0.5, 1.0); |
| WebGLEngine.create({ canvas: "canvas", physics: new PhysXPhysics() }).then((engine) => { | ||
| engine.canvas.resizeByClientSize(); | ||
| const scene = engine.sceneManager.activeScene; | ||
| const rootEntity = scene.createRootEntity(); | ||
|
|
||
| // init camera | ||
| const cameraEntity = rootEntity.createChild("camera"); | ||
| const camera = cameraEntity.addComponent(Camera); | ||
| const pos = cameraEntity.transform.position; | ||
| pos.set(10, 10, 10); | ||
| cameraEntity.transform.position = pos; | ||
| cameraEntity.transform.lookAt(new Vector3(0, 0, 0)); | ||
|
|
||
| // Initialize group for selection | ||
| const group = new Group(); | ||
|
|
||
| // add gizmo | ||
| const gizmoEntity = rootEntity.createChild("editor-gizmo"); | ||
| const gizmo = gizmoEntity.addComponent(Gizmo); | ||
| gizmo.init(camera, group); | ||
| gizmo.state = State.translate; | ||
|
|
||
| // init light | ||
| scene.ambientLight.diffuseSolidColor.set(1, 1, 1, 1); | ||
| scene.ambientLight.diffuseIntensity = 1.2; | ||
|
|
||
| // init cube | ||
| const cubeEntity = rootEntity.createChild("cube"); | ||
| const renderer = cubeEntity.addComponent(MeshRenderer); | ||
| const mtl = new BlinnPhongMaterial(engine); | ||
| const color = mtl.baseColor; | ||
| color.r = 0.0; | ||
| color.g = 0.8; | ||
| color.b = 0.5; | ||
| color.a = 1.0; | ||
| renderer.mesh = PrimitiveMesh.createCuboid(engine); | ||
| renderer.setMaterial(mtl); | ||
| group.addEntity(cubeEntity); | ||
|
|
||
| engine.run(); | ||
| }); |
There was a problem hiding this comment.
Add error handling for the promise returned by WebGLEngine.create
Currently, if WebGLEngine.create fails (e.g., due to a missing canvas element or initialization issues), the error is unhandled. Adding a .catch block would improve the robustness of the application.
Apply this diff to add error handling:
WebGLEngine.create({ canvas: "canvas", physics: new PhysXPhysics() }).then((engine) => {
// existing code...
engine.run();
+}).catch((error) => {
+ console.error("Failed to create WebGLEngine:", error);
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| WebGLEngine.create({ canvas: "canvas", physics: new PhysXPhysics() }).then((engine) => { | |
| engine.canvas.resizeByClientSize(); | |
| const scene = engine.sceneManager.activeScene; | |
| const rootEntity = scene.createRootEntity(); | |
| // init camera | |
| const cameraEntity = rootEntity.createChild("camera"); | |
| const camera = cameraEntity.addComponent(Camera); | |
| const pos = cameraEntity.transform.position; | |
| pos.set(10, 10, 10); | |
| cameraEntity.transform.position = pos; | |
| cameraEntity.transform.lookAt(new Vector3(0, 0, 0)); | |
| // Initialize group for selection | |
| const group = new Group(); | |
| // add gizmo | |
| const gizmoEntity = rootEntity.createChild("editor-gizmo"); | |
| const gizmo = gizmoEntity.addComponent(Gizmo); | |
| gizmo.init(camera, group); | |
| gizmo.state = State.translate; | |
| // init light | |
| scene.ambientLight.diffuseSolidColor.set(1, 1, 1, 1); | |
| scene.ambientLight.diffuseIntensity = 1.2; | |
| // init cube | |
| const cubeEntity = rootEntity.createChild("cube"); | |
| const renderer = cubeEntity.addComponent(MeshRenderer); | |
| const mtl = new BlinnPhongMaterial(engine); | |
| const color = mtl.baseColor; | |
| color.r = 0.0; | |
| color.g = 0.8; | |
| color.b = 0.5; | |
| color.a = 1.0; | |
| renderer.mesh = PrimitiveMesh.createCuboid(engine); | |
| renderer.setMaterial(mtl); | |
| group.addEntity(cubeEntity); | |
| engine.run(); | |
| }); | |
| WebGLEngine.create({ canvas: "canvas", physics: new PhysXPhysics() }).then((engine) => { | |
| engine.canvas.resizeByClientSize(); | |
| const scene = engine.sceneManager.activeScene; | |
| const rootEntity = scene.createRootEntity(); | |
| // init camera | |
| const cameraEntity = rootEntity.createChild("camera"); | |
| const camera = cameraEntity.addComponent(Camera); | |
| const pos = cameraEntity.transform.position; | |
| pos.set(10, 10, 10); | |
| cameraEntity.transform.position = pos; | |
| cameraEntity.transform.lookAt(new Vector3(0, 0, 0)); | |
| // Initialize group for selection | |
| const group = new Group(); | |
| // add gizmo | |
| const gizmoEntity = rootEntity.createChild("editor-gizmo"); | |
| const gizmo = gizmoEntity.addComponent(Gizmo); | |
| gizmo.init(camera, group); | |
| gizmo.state = State.translate; | |
| // init light | |
| scene.ambientLight.diffuseSolidColor.set(1, 1, 1, 1); | |
| scene.ambientLight.diffuseIntensity = 1.2; | |
| // init cube | |
| const cubeEntity = rootEntity.createChild("cube"); | |
| const renderer = cubeEntity.addComponent(MeshRenderer); | |
| const mtl = new BlinnPhongMaterial(engine); | |
| const color = mtl.baseColor; | |
| color.r = 0.0; | |
| color.g = 0.8; | |
| color.b = 0.5; | |
| color.a = 1.0; | |
| renderer.mesh = PrimitiveMesh.createCuboid(engine); | |
| renderer.setMaterial(mtl); | |
| group.addEntity(cubeEntity); | |
| engine.run(); | |
| }).catch((error) => { | |
| console.error("Failed to create WebGLEngine:", error); | |
| }); |
| import { defineConfig } from "vite"; | ||
|
|
||
| const templateStr = fs.readFileSync(path.join(__dirname, "index.html"), "utf8"); | ||
| const platforms = fs.readdirSync(path.join(__dirname, "cases")); |
There was a problem hiding this comment.
Filter 'cases' directory entries to only process TypeScript files
Currently, the code processes all entries in the cases directory, which may include non-TypeScript files or directories. To ensure that only TypeScript files are processed, filter the entries to include only .ts files.
Apply the following diff to filter the platforms array:
-const platforms = fs.readdirSync(path.join(__dirname, "cases"));
+const platforms = fs
+ .readdirSync(path.join(__dirname, "cases"))
+ .filter((file) => path.extname(file) === ".ts");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const platforms = fs.readdirSync(path.join(__dirname, "cases")); | |
| const platforms = fs | |
| .readdirSync(path.join(__dirname, "cases")) | |
| .filter((file) => path.extname(file) === ".ts"); |
Please check if the PR fulfills these requirements
Summary by CodeRabbit
New Features
Bug Fixes
.gitignoreentries for better directory management.Documentation
package.jsonfiles to reflect new dependencies and scripts for testing.Tests
Chores
e2edirectory for testing.