chore(server): add test case for add field using integration api #1266
Conversation
WalkthroughThe changes involve updates to the integration tests for schema and field APIs in the Changes
Poem
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 (
|
✅ Deploy Preview for reearth-cms canceled.
|
yk-eukarya
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
server/e2e/integration_schema_test.go (1)
Line range hint
201-232: Update field type from "bool" to "text"In the test case starting at line 201, a field of type
"bool"is being created. According to the AI-generated summary, there was a change from"bool"to"text"in one of the test cases. Ensure that this change is reflected in the code if it's intended.If the field type should be
"text", apply the following change:"WithJSON(map[string]interface{}{ "key": "fKey1", - "type": "bool", + "type": "text", "multiple": false, "required": false, })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- server/e2e/integration_schema_test.go (2 hunks)
🧰 Additional context used
🔇 Additional comments (5)
server/e2e/integration_schema_test.go (5)
180-199: Verify the correctness of model assertions after field modificationsAfter adding or modifying fields, the test asserts various properties of the model. Ensure that all expected changes to the model are accurately reflected in these assertions, including the updated schema and field details.
Also applies to: 232-232
164-199: Ensure compliance with API documentationVerify that the API endpoints and payload structures used in the tests align with the API documentation. This includes parameter names, expected responses, and error codes. Consistency between tests and documentation helps prevent integration issues.
Also applies to: 201-232
180-199:⚠️ Potential issueEnsure consistent field count after field creation
In the assertions starting at line 180, the response object's
"schema"'s"fields"array length is checked to be equal to3. Given that a new field has been added, it would be valuable to verify that this count reflects the expected number of fields after each addition.Consider updating the expected length based on the initial state of the schema and how many fields have been added in prior test steps.
232-232:⚠️ Potential issueDouble-check array length assertion
At line 232, there is an assertion checking that the
"fields"array length is equal to4. Similar to the earlier comment, ensure that this expected value accurately reflects the number of fields after the operations performed in the test.
164-199: Clarify the use of non-ASCII characters in the "key" fieldIn the test case starting at line 164, the
"key"field is set to"テスト", which contains non-ASCII characters. While this may be intentional for testing internationalization support, it's important to ensure that the system correctly handles such inputs.Consider adding assertions to verify that the system behaves as expected when using non-ASCII characters. Also, ensure that the rest of the system (e.g., database schemas, APIs) supports Unicode characters in keys.
Run the following script to search for any potential issues with non-ASCII keys in the codebase:
✅ Verification successful
Non-ASCII characters in the "key" field are correctly handled
The only occurrence of a non-ASCII
"key"value is inserver/e2e/integration_schema_test.golines 164-199, which is intentional for testing internationalization support. No additional instances or validation constraints related to the"key"field were found in the codebase, confirming that the system correctly handles non-ASCII characters in keys.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for usages of the "key" field with non-ASCII characters. # Test: Find all occurrences where "key" is set with non-ASCII characters. rg --type go --pcre2 'key":\s*"[^"\x00-\x7F]+'Length of output: 108
Script:
#!/bin/bash # Description: # 1. Find all struct definitions with a "Key" field and check their types. # 2. Search for all test cases assigning non-ASCII characters to the "key" field. # 3. Verify API handlers that process the "key" field for any validation constraints. # 1. Find struct definitions with a "Key" field ast-grep --lang go --pattern 'type $_Struct { $$$ Key $_Field $_Type $$$ }' # 2. Search for test cases assigning non-ASCII characters to "key" rg --type go --pcre2 '"key":\s*"[^"\x00-\x7F]+"' # 3. Check API handlers for "key" field validations rg --type go 'key.*validation|key.*unicode|key.*utf'Length of output: 734
Script:
#!/bin/bash # Description: # 1. Refine struct definitions search to avoid multiple AST nodes error. # 2. Re-confirm if there are additional test cases assigning non-ASCII characters to "key". # 3. Broaden search for any validations or processing related to the "key" field. # 1. Refined search for struct definitions with a "Key" field ast-grep --lang go --pattern 'type $_Struct struct { $$$ Key $_Field $_Type $$$ }' # 2. Re-search for test cases assigning non-ASCII characters to "key" rg --type go --pcre2 '"key":\s*"[^"\x00-\x7F]+"' # 3. Broaden search for any "key" field validations or Unicode handling rg --type go '(key.*validation)|(key.*unicode)|(key.*utf)'Length of output: 256
Script:
#!/bin/bash # Description: # 1. Refined search for struct definitions with a "Key" field to avoid multiple AST nodes error. # 2. Re-confirm if there are additional test cases assigning non-ASCII characters to "key". # 3. Broaden search for any validations or processing related to the "key" field. # 1. Refined search for struct definitions with a "Key" field ast-grep --lang go --pattern 'type $_Struct struct { ... Key $_Field $_Type ... }' # 2. Re-search for test cases assigning non-ASCII characters to "key" rg --type go --pcre2 '"key":\s*"[^"\x00-\x7F]+"' # 3. Broaden search for any "key" field validations or Unicode handling rg --type go '(key.*validation)|(key.*unicode)|(key.*utf)'Length of output: 272
Script:
#!/bin/bash # Description: # 1. Use rg to find all struct definitions containing the "Key" field. # 2. Re-confirm if there are additional test cases assigning non-ASCII characters to "key". # 3. Ensure there are no validation constraints or Unicode handling for the "Key" field. # 1. Find all struct definitions with a "Key" field rg 'type\s+\w+\s+struct\s+\{[^}]*Key\s+\w+\s+\w+' --type go # 2. Re-search for test cases assigning non-ASCII characters to "key" rg --type go --pcre2 '"key":\s*"[^"\x00-\x7F]+"' # 3. Check for any "Key" field validations or Unicode handling rg --type go '(Key.*validation)|(Key.*unicode)|(Key.*utf)'Length of output: 233
| WithHeader("authorization", "Bearer "+secret). | ||
| WithJSON(map[string]interface{}{ | ||
| "key": "テスト", | ||
| "type": "text", | ||
| "multiple": false, | ||
| "required": false, | ||
| }). | ||
| Expect(). | ||
| Status(http.StatusOK). | ||
| JSON(). | ||
| Object() | ||
|
|
||
| res.ContainsKey("id") | ||
|
|
||
| res = e.GET("/api/models/{modelId}", mId1). | ||
| WithHeader("authorization", "Bearer "+secret). | ||
| Expect(). | ||
| Status(http.StatusOK). | ||
| JSON(). | ||
| Object() | ||
|
|
||
| res.HasValue("id", mId1.String()). | ||
| HasValue("name", "m1"). | ||
| HasValue("description", "m1 desc"). | ||
| HasValue("public", true). | ||
| HasValue("key", ikey1.String()). | ||
| HasValue("projectId", pid). | ||
| HasValue("schemaId", sid1) | ||
|
|
||
| res.Value("schema").Object().Value("fields").Array().Length().IsEqual(3) | ||
| res.Value("createdAt").NotNull() | ||
| res.Value("updatedAt").NotNull() | ||
| res.Value("lastModified").NotNull() | ||
| // endregion |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider refactoring repetitive code with helper functions
The test code contains repetitive blocks for creating fields and asserting responses. To improve maintainability and readability, consider extracting common operations into helper functions.
For example, you can create a helper function for field creation:
func createField(e *httpexpect.Expect, endpoint string, schemaID string, fieldData map[string]interface{}) *httpexpect.Object {
return e.POST(endpoint, schemaID).
WithHeader("authorization", "Bearer "+secret).
WithJSON(fieldData).
Expect().
Status(http.StatusOK).
JSON().
Object()
}And a helper for retrieving and asserting model details:
func getModel(e *httpexpect.Expect, modelID string) *httpexpect.Object {
return e.GET("/api/models/{modelId}", modelID).
WithHeader("authorization", "Bearer "+secret).
Expect().
Status(http.StatusOK).
JSON().
Object()
}Using these helpers will reduce code duplication and make the tests clearer.
Also applies to: 201-232
| // region text | ||
| res := e.POST(endpoint, sid1). | ||
| WithHeader("authorization", "Bearer "+secret). | ||
| WithJSON(map[string]interface{}{ | ||
| "key": "テスト", | ||
| "type": "text", | ||
| "multiple": false, | ||
| "required": false, | ||
| }). | ||
| Expect(). | ||
| Status(http.StatusOK). | ||
| JSON(). | ||
| Object() | ||
|
|
||
| res.ContainsKey("id") | ||
|
|
||
| res = e.GET("/api/models/{modelId}", mId1). | ||
| WithHeader("authorization", "Bearer "+secret). | ||
| Expect(). | ||
| Status(http.StatusOK). | ||
| JSON(). | ||
| Object() | ||
|
|
||
| res.HasValue("id", mId1.String()). | ||
| HasValue("name", "m1"). | ||
| HasValue("description", "m1 desc"). | ||
| HasValue("public", true). | ||
| HasValue("key", ikey1.String()). | ||
| HasValue("projectId", pid). | ||
| HasValue("schemaId", sid1) | ||
|
|
||
| res.Value("schema").Object().Value("fields").Array().Length().IsEqual(3) | ||
| res.Value("createdAt").NotNull() | ||
| res.Value("updatedAt").NotNull() | ||
| res.Value("lastModified").NotNull() | ||
| // endregion |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add tests for error handling and edge cases
Current tests focus on successful field creation. Consider adding test cases to cover error scenarios, such as:
- Attempting to create a field with an existing key.
- Creating a field with invalid data types.
- Handling of required fields when not provided.
This will enhance the robustness of the test suite.
Also applies to: 201-232
Summary by CodeRabbit
Bug Fixes
New Features