Conversation
Add comprehensive test coverage for: - Unicode support in field names and values (emoji, Hindi, Chinese, Korean, Japanese, Arabic, European special characters) - Unicode in queries and aggregation pipelines - $function operator parsing with various JS function patterns - $accumulator operator parsing with init, accumulate, merge, finalize Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive test coverage for Unicode support and JavaScript function parsing in MongoDB shell command execution. The tests verify that the parser correctly handles Unicode characters in field names and values, as well as JavaScript code in aggregation operators.
Changes:
- Add 31 Unicode support test cases covering emoji, Hindi, Chinese, Korean, Japanese, Arabic, and special characters in field values, field names, queries, and aggregation operations
- Add 8 JavaScript function parsing test cases for
$functionand$accumulatoroperators with various JavaScript code patterns including conditionals, array operations, and special characters
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| require.Contains(t, result.Rows[0], `"acknowledged": true`) | ||
|
|
||
| // Verify the data was stored correctly by querying it back | ||
| findStmt := `db.unicode.findOne({` + tt.checkKey + `: "` + tt.checkVal + `"})` |
There was a problem hiding this comment.
String concatenation is used to build the query, which could be fragile if the Unicode values contain special characters like quotes or backslashes that need escaping. Consider using a more robust approach to build the findOne query, such as using a template or ensuring proper escaping of the Unicode values before concatenation.
| require.Contains(t, result.Rows[0], `"acknowledged": true`) | ||
|
|
||
| // Query using unicode field name | ||
| findStmt := `db.unicode_fields.findOne({"` + tt.fieldName + `": "` + tt.fieldVal + `"})` |
There was a problem hiding this comment.
String concatenation is used to build the query with Unicode field names, which could be fragile if the Unicode values contain special characters like quotes or backslashes that need escaping. Consider using a more robust approach to build the findOne query or ensuring proper escaping of the Unicode values before concatenation.
| if err != nil { | ||
| // MongoDB returns "JavaScript execution is disabled" or similar | ||
| // when JS is disabled, which is fine - it means parsing succeeded. | ||
| errStr := err.Error() | ||
| require.NotContains(t, errStr, "parse", "statement should parse without errors") | ||
| require.NotContains(t, errStr, "syntax", "statement should have valid syntax") | ||
| } |
There was a problem hiding this comment.
The error checking logic only verifies that parse-related error strings are absent when an error occurs, but doesn't verify the actual success case. This approach is fragile because it assumes any non-parse error means parsing succeeded. Consider using more explicit error type checking (like checking for gomongo.ParseError) or documenting this behavior more clearly if this is intentional for testing parsing when JavaScript execution may be disabled.
| if err != nil { | ||
| errStr := err.Error() | ||
| require.NotContains(t, errStr, "parse", "statement should parse without errors") | ||
| require.NotContains(t, errStr, "syntax", "statement should have valid syntax") | ||
| } |
There was a problem hiding this comment.
The error checking logic only verifies that parse-related error strings are absent when an error occurs, but doesn't verify the actual success case. This approach is fragile because it assumes any non-parse error means parsing succeeded. Consider using more explicit error type checking (like checking for gomongo.ParseError) or documenting this behavior more clearly if this is intentional for testing parsing when JavaScript execution may be disabled.
| if err != nil { | ||
| errStr := err.Error() | ||
| require.NotContains(t, errStr, "parse", "statement should parse without errors") | ||
| require.NotContains(t, errStr, "syntax", "statement should have valid syntax") | ||
| } |
There was a problem hiding this comment.
The error checking logic only verifies that parse-related error strings are absent when an error occurs, but doesn't verify the actual success case. This approach is fragile because it assumes any non-parse error means parsing succeeded. Consider using more explicit error type checking (like checking for gomongo.ParseError) or documenting this behavior more clearly if this is intentional for testing parsing when JavaScript execution may be disabled.
1 participant
Summary
$functionand$accumulatoroperatorsTest Coverage
Unicode Support (
TestUnicodeSupport)JavaScript Function Parsing (
TestAggregateJavaScriptFunctionParsing)$functionoperator: simple functions, multiple arguments, conditional logic, array operations, use in $match via $expr$accumulatoroperator: basic accumulator, with finalize, with initArgs, multiple accumulateArgsTest plan
🤖 Generated with Claude Code