-
-
Notifications
You must be signed in to change notification settings - Fork 7
Added knex integration tests for dates inc relative #29
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -413,4 +413,46 @@ describe('Integration with Mingo', function () { | |||||
| query.queryJSON(advancedJSON.posts[4]).should.eql(false); | ||||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| describe.only('Dates', function () { | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: fd -t f "nql_mingo.test.js" | head -5Repository: TryGhost/NQL Length of output: 103 🏁 Script executed: # Check if the file exists and read the relevant section
wc -l packages/nql/test/integration/nql_mingo.test.jsRepository: TryGhost/NQL Length of output: 107 🏁 Script executed: # Read the specific lines around the describe.only
sed -n '410,460p' packages/nql/test/integration/nql_mingo.test.jsRepository: TryGhost/NQL Length of output: 2589 Remove the exclusive Mocha suite before merge.
Suggested fix- describe.only('Dates', function () {
+ describe('Dates', function () {- console.log(new Date().toISOString());
- console.log(new Date());📝 Committable suggestion
Suggested change
🧰 Tools🪛 ESLint[error] 417-417: Unexpected exclusive mocha test. (ghost/mocha/no-exclusive-tests) 🤖 Prompt for AI Agents |
||||||
| // Dates are a nightmare because SQLite3 and MySQL work differently, and knex doesn't help here | ||||||
| // The date format that goes into a database has to be YYYY-MM-DD HH:mm:ss, but what comes out is normalised to ISO YYYY-MM-DDTHH:mm:ss.000Z | ||||||
|
|
||||||
| it('can query JSON by date', function () { | ||||||
| const query = makeQuery('created_at:>=\'2022-03-02 10:14:23\''); | ||||||
|
|
||||||
| query.queryJSON({created_at: '2022-03-02T10:14:23.000Z'}).should.eql(true); | ||||||
| query.queryJSON({created_at: '2022-03-02 10:14:23'}).should.eql(true); | ||||||
| query.queryJSON({created_at: '2022-03-02T10:14:24.000Z'}).should.eql(true); | ||||||
| query.queryJSON({created_at: '2022-03-02 10:14:24'}).should.eql(true); | ||||||
| // FAIL query.queryJSON({created_at: '2022-03-02T10:14:22.000Z'}).should.eql(false); | ||||||
| query.queryJSON({created_at: '2022-03-02 10:14:22'}).should.eql(false); | ||||||
|
|
||||||
| query.queryJSON({}).should.eql(false); | ||||||
| }); | ||||||
|
|
||||||
| it('can query JSON by ISO date', function () { | ||||||
| const query = makeQuery('created_at:>=\'2022-03-02T10:14:23.000Z\''); | ||||||
|
|
||||||
| query.queryJSON({created_at: '2022-03-02T10:14:23.000Z'}).should.eql(true); | ||||||
| // FAIL query.queryJSON({created_at: '2022-03-02 10:14:23'}).should.eql(true); | ||||||
| query.queryJSON({created_at: '2022-03-02T10:14:24.000Z'}).should.eql(true); | ||||||
| // FAIL query.queryJSON({created_at: '2022-03-02 10:14:24'}).should.eql(true); | ||||||
| query.queryJSON({created_at: '2022-03-02T10:14:22.000Z'}).should.eql(false); | ||||||
| query.queryJSON({created_at: '2022-03-02 10:14:22'}).should.eql(false); | ||||||
|
|
||||||
| query.queryJSON({}).should.eql(false); | ||||||
| }); | ||||||
|
|
||||||
| it('can query JSON by rel date', function () { | ||||||
| const query = makeQuery('created_at:>=now-1d'); | ||||||
|
|
||||||
| console.log(new Date().toISOString()); | ||||||
| console.log(new Date()); | ||||||
|
Comment on lines
+450
to
+451
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Drop debug console output from tests. Line 450 and Line 451 introduce Suggested fix- console.log(new Date().toISOString());
- console.log(new Date());📝 Committable suggestion
Suggested change
🧰 Tools🪛 ESLint[error] 450-450: Unexpected console statement. (no-console) [error] 451-451: Unexpected console statement. (no-console) 🤖 Prompt for AI Agents |
||||||
|
|
||||||
| query.queryJSON({created_at: new Date().toISOString()}).should.eql(true); | ||||||
| query.queryJSON({created_at: '2022-01-00T00:00:00.000Z'}).should.eql(false); | ||||||
| query.queryJSON({created_at: '2022-01-00 00:00:00'}).should.eql(false); | ||||||
| }); | ||||||
| }); | ||||||
| }); | ||||||
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.
Make row ordering explicit before index-based assertions.
These tests assert
result[0..n]but the query has no explicit sort, so ordering is nondeterministic and may flake across DB engines.Suggested fix
Also applies to: 85-92, 100-109
🤖 Prompt for AI Agents