Repository navigation
fix(queries): keep comments out of generated AQL - #1001
Conversation
|
| expect(multidevice.join('\n')).not.toMatch(/^\s*\/\//m); | ||
| expect(context.join('\n')).not.toMatch(/^\s*\/\//m); |
There was a problem hiding this comment.
Comment check misses other forms
The new assertions catch // only at the start of a line. An inline // comment or a /* ... */ comment in generated AQL would pass the test, and querystr_to_array would still send it as part of a statement. This leaves a gap in the regression check. Please cover those forms without matching comment-like text inside string literals.
There was a problem hiding this comment.
Addressed in #1007: the test now strips string literals and checks for // and /* anywhere, across all query builders.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1001 +/- ##
==========================================
+ Coverage 57.66% 57.69% +0.03%
==========================================
Files 51 51
Lines 3231 3231
Branches 794 754 -40
==========================================
+ Hits 1863 1864 +1
- Misses 1291 1351 +60
+ Partials 77 16 -61 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
* test(queries): catch inline and block comments in generated AQL The check added in #1001 only matched // at the start of a line, so an inline // or a /* */ comment would still slip into a generated query and break it (#999). Strip string literals first, so comment-like text inside them (such as https:// in a category regex) is allowed, and check every query builder with realistic params. Addresses the Greptile review on #1001. * test(activity): check active history stays aligned across request chunks Return distinct data per period so a period paired with another period's result would fail, not just a missing one. Follow-up to review on #1003. * test(queries): use typed, tuple-shaped fixtures in the AQL comment check Addresses Greptile review on #1007.
Fixes #999.
With "Use multidevice query" enabled, Activity fails because the generated AQL contains
//comments. The activity-context query has the same problem.Move those explanations outside the query strings and add a regression test for both builders.
Reproduced with synthetic events on a local aw-server: Activity shows the syntax error before the change and loads after it. Both queries execute successfully after the fix.
Before / after (with a short blank gap):
activitywatch-aql-before-after.mp4