-
Notifications
You must be signed in to change notification settings - Fork 112
chore: organize test #156
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
chore: organize test #156
Conversation
Reviewer's GuideReorganizes the Cypress E2E test suite by introducing centralized support utilities and configuration, expanding selector abstractions in selectors.ts, and refactoring all spec files to use the new helper APIs for consistent, maintainable test code. Class diagram for new and updated selector helpers in selectors.tsclassDiagram
byTestId : string
byTestIdPrefix : string
byTestIdContains : string
class AuthSelectors {
emailInput()
magicLinkButton()
enterCodeManuallyButton()
otpCodeInput()
otpSubmitButton()
passwordSignInButton()
passwordInput()
passwordSubmitButton()
}
class WorkspaceSelectors {
dropdownTrigger()
dropdownContent()
}
class AddPageSelectors {
inlineAddButton()
addAIChatButton()
}
class ModelSelectorSelectors {
button()
options()
}
class SpaceSelectors {
moreActionsButton()
}
byTestId <|-- AuthSelectors
byTestId <|-- WorkspaceSelectors
byTestId <|-- AddPageSelectors
byTestId <|-- ModelSelectorSelectors
byTestId <|-- SpaceSelectors
byTestIdPrefix <|-- ModelSelectorSelectors
byTestIdContains <|-- ModelSelectorSelectors
Class diagram for new centralized Cypress support modulesclassDiagram
class chat-mocks {
+setupChatApiStubs(options)
+mockChatMessage(content, messageId, authorType)
+mockEmptyChatMessages()
+mockChatSettings(aiModel, ragIds)
+mockUpdateChatSettings()
+mockModelList(modelNames, defaultModel)
+mockRelatedQuestions(messageId, questions)
+mockSendMessage(responseContent)
+mockChatStreaming(chunks)
+mockChatError(errorMessage)
DEFAULT_MESSAGE_ID : number
DEFAULT_MESSAGE_CONTENT : string
}
class api-mocks {
+createAuthResponse(email, accessToken, refreshToken, userId)
+mockAuthEndpoints(email, accessToken, refreshToken, userId)
+mockOTPEndpoints(email, accessToken, refreshToken, userId)
+mockWorkspaceEndpoints(workspaceId, userId, workspaceName)
+mockUserVerification(accessToken, isNewUser)
+mockCompleteAuthFlow(email, accessToken, refreshToken, userId, workspaceId)
}
class exception-handlers {
+setupCommonExceptionHandlers(additionalPatterns)
+ignoreAllExceptions()
IGNORED_ERROR_PATTERNS : string[]
}
class test-config {
TestConfig : object
generateRandomEmail()
logAppFlowyEnvironment()
}
class test-helpers {
// (methods not shown, placeholder for future helpers)
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- Consider adopting a single, consistent pattern for querying
data-testidelements (either the centralized selectors in selectors.ts or the byTestId helper) across all specs to avoid mixing raw strings and improve maintainability. - Double-check that no direct cy.get('[data-testid="…"]') calls remain by grepping for raw test IDs—every selector should ideally come from your support selectors or byTestId functions.
- To reduce repetitive imports, you could register common utilities (TestConfig, exception handlers, test-helpers) in cypress/support/index.ts so individual specs don’t need to import them explicitly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adopting a single, consistent pattern for querying `data-testid` elements (either the centralized selectors in selectors.ts or the byTestId helper) across all specs to avoid mixing raw strings and improve maintainability.
- Double-check that no direct cy.get('[data-testid="…"]') calls remain by grepping for raw test IDs—every selector should ideally come from your support selectors or byTestId functions.
- To reduce repetitive imports, you could register common utilities (TestConfig, exception handlers, test-helpers) in cypress/support/index.ts so individual specs don’t need to import them explicitly.
## Individual Comments
### Comment 1
<location> `cypress/support/chat-mocks.ts:35` </location>
<code_context>
+ * @param messageId - Message ID (default: 101)
+ * @param authorType - Author type (3 = assistant, 1 = user)
+ */
+export const mockChatMessage = (
+ content = DEFAULT_MESSAGE_CONTENT,
+ messageId = DEFAULT_MESSAGE_ID,
</code_context>
<issue_to_address>
**issue (complexity):** Consider introducing a factory helper to centralize and reduce repetitive cy.intercept logic in your mock endpoint definitions.
Consider extracting a small factory to remove the 90% duplication in your `cy.intercept` calls. For example, you can add a helper like this at the top of `chat-mocks.ts`:
```ts
type MockOpts<Args> = {
method: 'GET' | 'POST' | 'PATCH'
url: string
alias: string
toBody: (args: Args) => unknown
}
function createMockEndpoint<Args>(opts: MockOpts<Args>) {
return (args: Args) => {
cy.intercept(opts.method, opts.url, {
statusCode: 200,
body: opts.toBody(args),
}).as(opts.alias)
}
}
// common success wrapper
const success = <T>(data: T) => ({ code: 0, message: 'success', data })
```
Then you can rewrite your existing mocks in just one line each:
```ts
export const mockChatSettings =
createMockEndpoint<{ aiModel: string; ragIds: string[] }>({
method: 'GET',
url: '**/api/chat/**/settings**',
alias: 'getChatSettings',
toBody: ({ aiModel, ragIds }) =>
success({ rag_ids: ragIds, metadata: { ai_model: aiModel } }),
})
export const mockUpdateChatSettings =
createMockEndpoint<{}>({
method: 'PATCH',
url: '**/api/chat/**/settings**',
alias: 'updateChatSettings',
toBody: () => ({ code: 0, message: 'success' }), // no data
})
```
You can apply the same pattern to all your other mocks, or split them into separate modules (messages, settings, models) to keep each file small and focused.
</issue_to_address>
### Comment 2
<location> `cypress/support/api-mocks.ts:48` </location>
<code_context>
+ * Mocks standard authentication endpoints (password login, verify, refresh)
+ * Returns the generated IDs and tokens for use in tests
+ */
+export const mockAuthEndpoints = (
+ email: string,
+ accessToken = `mock-token-${uuidv4()}`,
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring repeated cy.intercept boilerplate into small helper functions to simplify and shorten each mock function.
```markdown
You still have quite a bit of repeated “cy.intercept(…, {…}).as(…)” boilerplate for verify-user, refresh-token and workspace payloads. Pulling those into tiny helpers will make each mock-fn just a few lines long:
```ts
// support/api-mocks.ts
// 1) small wrapper to reduce intercept boilerplate
function setupIntercept(
method: Cypress.RouteMatcher['method'],
url: string,
alias: string,
body: unknown,
) {
cy.intercept(method, url, { statusCode: 200, body }).as(alias)
}
// 2) verify-user is identical across many mocks
function interceptVerifyUser(apiUrl: string, accessToken: string, isNew = false) {
setupIntercept(
'GET',
`${apiUrl}/api/user/verify/${accessToken}`,
'verifyUser',
{ code: 0, data: { is_new: isNew }, message: 'User verified successfully' },
)
}
// 3) refresh-token is identical across auth & otp
function interceptRefresh(
gotrueUrl: string,
email: string,
accessToken: string,
refreshToken: string,
userId: string,
) {
setupIntercept(
'POST',
`${gotrueUrl}/token?grant_type=refresh_token`,
'refreshToken',
createAuthResponse(email, accessToken, refreshToken, userId),
)
}
// 4) workspace payload repeats itself: factor it out
function makeWorkspacePayload(workspaceId: string, workspaceName: string, userId: string) {
const ws = {
workspace_id: workspaceId,
workspace_name: workspaceName,
icon: '',
created_at: Date.now().toString(),
database_storage_id: '',
owner_uid: 1,
owner_name: 'Test User',
member_count: 1,
}
return {
code: 0,
data: {
user_profile: { uuid: userId },
visiting_workspace: ws,
workspaces: [ws],
},
}
}
// Now your mocks shrink to:
export const mockAuthEndpoints = (
email: string,
accessToken = `mock-token-${uuidv4()}`,
refreshToken = `mock-refresh-${uuidv4()}`,
userId = uuidv4(),
) => {
const { gotrueUrl, apiUrl } = TestConfig
setupIntercept(
'POST',
`${gotrueUrl}/token?grant_type=password`,
'passwordLogin',
createAuthResponse(email, accessToken, refreshToken, userId),
)
interceptVerifyUser(apiUrl, accessToken)
interceptRefresh(gotrueUrl, email, accessToken, refreshToken, userId)
return { userId, accessToken, refreshToken }
}
export const mockOTPEndpoints = (
email: string,
accessToken = `mock-otp-token-${uuidv4()}`,
refreshToken = `mock-otp-refresh-${uuidv4()}`,
userId = uuidv4(),
) => {
const { gotrueUrl, apiUrl } = TestConfig
setupIntercept('POST', `${gotrueUrl}/otp`, 'sendOTP', {})
setupIntercept(
'POST',
`${gotrueUrl}/verify`,
'verifyOTP',
createAuthResponse(email, accessToken, refreshToken, userId),
)
interceptVerifyUser(apiUrl, accessToken)
interceptRefresh(gotrueUrl, email, accessToken, refreshToken, userId)
return { userId, accessToken, refreshToken }
}
export const mockWorkspaceEndpoints = (
workspaceId = uuidv4(),
userId = uuidv4(),
workspaceName = 'Test Workspace',
) => {
const { apiUrl } = TestConfig
setupIntercept(
'GET',
`${apiUrl}/api/user/workspace`,
'getUserWorkspaceInfo',
makeWorkspacePayload(workspaceId, workspaceName, userId),
)
return { workspaceId, userId }
}
```
This removes most of the copy/paste and keeps each mock-fn under ~10 lines.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| * @param messageId - Message ID (default: 101) | ||
| * @param authorType - Author type (3 = assistant, 1 = user) | ||
| */ | ||
| export const mockChatMessage = ( |
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.
issue (complexity): Consider introducing a factory helper to centralize and reduce repetitive cy.intercept logic in your mock endpoint definitions.
Consider extracting a small factory to remove the 90% duplication in your cy.intercept calls. For example, you can add a helper like this at the top of chat-mocks.ts:
type MockOpts<Args> = {
method: 'GET' | 'POST' | 'PATCH'
url: string
alias: string
toBody: (args: Args) => unknown
}
function createMockEndpoint<Args>(opts: MockOpts<Args>) {
return (args: Args) => {
cy.intercept(opts.method, opts.url, {
statusCode: 200,
body: opts.toBody(args),
}).as(opts.alias)
}
}
// common success wrapper
const success = <T>(data: T) => ({ code: 0, message: 'success', data })Then you can rewrite your existing mocks in just one line each:
export const mockChatSettings =
createMockEndpoint<{ aiModel: string; ragIds: string[] }>({
method: 'GET',
url: '**/api/chat/**/settings**',
alias: 'getChatSettings',
toBody: ({ aiModel, ragIds }) =>
success({ rag_ids: ragIds, metadata: { ai_model: aiModel } }),
})
export const mockUpdateChatSettings =
createMockEndpoint<{}>({
method: 'PATCH',
url: '**/api/chat/**/settings**',
alias: 'updateChatSettings',
toBody: () => ({ code: 0, message: 'success' }), // no data
})You can apply the same pattern to all your other mocks, or split them into separate modules (messages, settings, models) to keep each file small and focused.
chore: organize test
Description
Checklist
General
Testing
Feature-Specific
Summary by Sourcery
Organize and standardize the Cypress E2E test suite by centralizing selectors, configuration, and utility functions, and refactor existing specs to leverage these new modules for improved consistency and maintainability.
Enhancements: