-
-
Notifications
You must be signed in to change notification settings - Fork 18
run tests in parallel #1460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
run tests in parallel #1460
Conversation
There was a problem hiding this 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 enables parallel test execution by transitioning from a shared test admin user to creating unique, isolated user instances for each test. The key changes support concurrent test runs without data collisions.
- Removed
--serialflag from thetest-allscript in package.json to enable parallel test execution - Refactored
registerUserAndReturnUserInfoto create unique users directly in the database for each test instead of logging into a shared admin account - Introduced
setupUserWithSecretshelper function to streamline test setup and reduce code duplication
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| backend/package.json | Removed --serial flag from test-all script to enable parallel test execution |
| backend/test/utils/register-user-and-return-user-info.ts | Refactored to create unique users with randomly generated credentials instead of reusing a shared admin account |
| backend/test/ava-tests/non-saas-tests/non-saas-secrets-e2e.test.ts | Added setupUserWithSecrets helper and updated tests to use unique secret slugs for test isolation |
| backend/test/ava-tests/non-saas-tests/non-saas-connection-e2e.test.ts | Added assertion to verify connection creation succeeded |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .set('masterpwd', 'ahalaimahalai') | ||
| .set('Content-Type', 'application/json') | ||
| .set('Accept', 'application/json'); | ||
|
|
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The setupUserWithSecrets helper function does not validate the response status when creating the connection. If the connection creation fails, the function will attempt to parse undefined or error text as JSON, causing a runtime error that's hard to debug.
Consider checking the response status:
const createdConnection = await request(app.getHttpServer())
.post('/connection')
.send(newConnection)
.set('Cookie', token)
.set('masterpwd', 'ahalaimahalai')
.set('Content-Type', 'application/json')
.set('Accept', 'application/json');
if (createdConnection.status !== 201) {
throw new Error(`Failed to create connection: ${createdConnection.text}`);
}
const connectionId = JSON.parse(createdConnection.text).id;| if (createdConnection.status !== 201) { | |
| throw new Error(`Failed to create connection: ${createdConnection.text}`); | |
| } |
| import { DataSource } from 'typeorm'; | ||
| import { Constants } from '../../src/helpers/constants/constants.js'; | ||
| import { TestUtils } from './test.utils.js'; | ||
| import { isSaaS } from '../../src/helpers/app/is-saas.js'; |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The isSaaS import is no longer used after refactoring the registerUserAndReturnUserInfo function to always create users directly in the database. Consider removing this unused import to keep the code clean.
| import { isSaaS } from '../../src/helpers/app/is-saas.js'; |
| await request(app.getHttpServer()) | ||
| .post('/secrets') | ||
| .set('Cookie', token) | ||
| .set('masterpwd', 'ahalaimahalai') | ||
| .set('Content-Type', 'application/json') | ||
| .set('Accept', 'application/json') | ||
| .send(secret); |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The setupUserWithSecrets helper function does not validate the response status when creating secrets. If the secret creation fails (e.g., due to validation errors or server issues), the test will continue without awareness of the failure, potentially causing confusing test results.
Consider checking the response status for each secret creation:
for (const secret of secrets) {
const response = await request(app.getHttpServer())
.post('/secrets')
.set('Cookie', token)
.set('masterpwd', 'ahalaimahalai')
.set('Content-Type', 'application/json')
.set('Accept', 'application/json')
.send(secret);
if (response.status !== 201) {
throw new Error(`Failed to create secret: ${response.text}`);
}
}| await request(app.getHttpServer()) | |
| .post('/secrets') | |
| .set('Cookie', token) | |
| .set('masterpwd', 'ahalaimahalai') | |
| .set('Content-Type', 'application/json') | |
| .set('Accept', 'application/json') | |
| .send(secret); | |
| const response = await request(app.getHttpServer()) | |
| .post('/secrets') | |
| .set('Cookie', token) | |
| .set('masterpwd', 'ahalaimahalai') | |
| .set('Content-Type', 'application/json') | |
| .set('Accept', 'application/json') | |
| .send(secret); | |
| if (response.status !== 201) { | |
| throw new Error(`Failed to create secret: ${response.text}`); | |
| } |
No description provided.