fix(api): changes in action.json reflection on action test #657
Conversation
✅ Deploy Preview for reearth-flow canceled.
|
WalkthroughThe changes in this pull request focus on enhancing the test cases within 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (6)
api/internal/app/actions_test.go (6)
37-41: Consider validating all actions in the responseWhile checking the first action's fields is good, consider extending these assertions to validate all actions in the response to ensure complete data quality.
- firstAction := response[0] - assert.NotEmpty(t, firstAction.Name) - assert.NotEmpty(t, firstAction.Type) - assert.NotEmpty(t, firstAction.Description) + for i, action := range response { + assert.NotEmpty(t, action.Name, "Action %d should have a name", i) + assert.NotEmpty(t, action.Type, "Action %d should have a type", i) + assert.NotEmpty(t, action.Description, "Action %d should have a description", i) + }
46-63: Enhance test coverage with additional edge casesThe test data setup is well-structured, but consider adding more test cases to cover:
- Actions with special characters in name/description
- Actions with empty fields
- Actions with very long text
actionsData = ActionsData{ Actions: []Action{ { Name: "FileWriter", Description: "Writes features to a file", Type: ActionTypeSink, Categories: []string{"File"}, }, { Name: "OtherAction", Description: "Some other action", Type: ActionTypeProcessor, }, + { + Name: "Special-Case!", + Description: "Action with special chars: @#$%", + Type: ActionTypeProcessor, + }, + { + Name: "", + Description: "Action with empty name", + Type: ActionTypeProcessor, + }, + { + Name: "VeryLongName" + strings.Repeat("x", 100), + Description: "Very long description" + strings.Repeat("y", 1000), + Type: ActionTypeProcessor, + }, }, }
80-82: Improve error message clarityThe current error message could be more descriptive to help debug test failures.
- assert.Contains(t, lowercaseContent, "file", - "Each result should contain 'file' in name or description") + assert.Contains(t, lowercaseContent, "file", + "Action '%s' (content: '%s') should contain 'file' in name or description", + action.Name, lowercaseContent)
87-105: Test nil vs empty categories arrayConsider adding test cases to verify correct handling of both nil and empty categories arrays, as they might be handled differently.
actionsData = ActionsData{ Actions: []Action{ { Name: "FileWriter", Type: ActionTypeSink, Description: "Writes features to a file", Categories: []string{"File"}, }, { Name: "Router", Type: ActionTypeProcessor, Description: "Action for last port forwarding for sub-workflows.", Categories: []string{}, }, + { + Name: "NilCategories", + Type: ActionTypeProcessor, + Description: "Action with nil categories", + Categories: nil, + }, }, }
120-136: Refactor action search into a helper functionConsider extracting the action search logic into a helper function for better reusability and readability.
+func findActionByName(actions []ActionSummary, name string) bool { + for _, action := range actions { + if action.Name == name { + return true + } + } + return false +} assert.NotEmpty(t, response.ByCategory) assert.NotEmpty(t, response.ByType) assert.Contains(t, response.ByCategory, "File") assert.Contains(t, response.ByCategory, "Uncategorized") assert.Contains(t, response.ByType, string(ActionTypeSink)) assert.Contains(t, response.ByType, string(ActionTypeProcessor)) uncategorizedActions := response.ByCategory["Uncategorized"] -routerFound := false -for _, action := range uncategorizedActions { - if action.Name == "Router" { - routerFound = true - break - } -} -assert.True(t, routerFound, "Router should be in Uncategorized category") +assert.True(t, findActionByName(uncategorizedActions, "Router"), + "Router should be in Uncategorized category")
169-172: Enhance assertion error messagesAdd more descriptive error messages to help identify which field failed validation.
- assert.Equal(t, testAction.Name, response.Name) - assert.Equal(t, testAction.Description, response.Description) - assert.Equal(t, testAction.Type, response.Type) - assert.Equal(t, testAction.Categories, response.Categories) + assert.Equal(t, testAction.Name, response.Name, + "Action name mismatch") + assert.Equal(t, testAction.Description, response.Description, + "Action description mismatch") + assert.Equal(t, testAction.Type, response.Type, + "Action type mismatch") + assert.Equal(t, testAction.Categories, response.Categories, + "Action categories mismatch")
| originalData := actionsData | ||
| defer func() { actionsData = originalData }() | ||
|
|
||
| var actionList []ActionSummary | ||
| err = json.Unmarshal(listRec.Body.Bytes(), &actionList) | ||
| require.NoError(t, err) | ||
| require.NotEmpty(t, actionList, "No actions found in the list") | ||
| testAction := Action{ | ||
| Name: "TestAction", | ||
| Type: ActionTypeProcessor, | ||
| Description: "Test action description", | ||
| Categories: []string{"TestCategory"}, | ||
| } | ||
|
|
||
| firstAction := actionList[0] | ||
| actionsData = ActionsData{ | ||
| Actions: []Action{testAction}, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for error cases
Consider adding test cases for various error scenarios:
- Action with invalid/malformed fields
- Action with extremely long values
- Action with special characters in name
testAction := Action{
Name: "TestAction",
Type: ActionTypeProcessor,
Description: "Test action description",
Categories: []string{"TestCategory"},
}
+// Add test cases for error scenarios
+errorActions := []Action{
+ {
+ Name: "", // Invalid empty name
+ Type: ActionTypeProcessor,
+ Description: "Action with empty name",
+ },
+ {
+ Name: strings.Repeat("x", 1000), // Very long name
+ Type: ActionTypeProcessor,
+ Description: "Action with very long name",
+ },
+ {
+ Name: "Test/Action", // Special characters
+ Type: ActionTypeProcessor,
+ Description: "Action with special characters",
+ },
+}
+
+for _, action := range errorActions {
+ t.Run("Error case: "+action.Name, func(t *testing.T) {
+ actionsData = ActionsData{
+ Actions: []Action{action},
+ }
+ // Test implementation here
+ })
+}Committable suggestion skipped: line range outside the PR's diff.
Overview
What I've done
What I haven't done
How I tested
Screenshot
Which point I want you to review particularly
Memo
Summary by CodeRabbit
Bug Fixes
Tests