feat: add safe empty where in plugin #925
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
igalklebanov
added
enhancement
src/plugin/safe-empty-array-where-in/safe-empty-array-where-in.transformer.ts
Outdated
Show resolved
Hide resolved
src/plugin/safe-empty-array-where-in/safe-empty-array-where-in.transformer.ts
Outdated
Show resolved
Hide resolved
src/plugin/safe-empty-array-where-in/safe-empty-array-where-in.transformer.ts
Outdated
Show resolved
Hide resolved
src/plugin/safe-empty-array-where-in/safe-empty-array-where-in.transformer.ts
Outdated
Show resolved
Hide resolved
src/plugin/safe-empty-array-where-in/safe-empty-array-where-in.transformer.ts
Outdated
Show resolved
Hide resolved
src/plugin/safe-empty-array-where-in/safe-empty-array-where-in.transformer.ts
Outdated
Show resolved
Hide resolved
src/plugin/safe-empty-array-where-in/safe-empty-array-where-in.transformer.ts
Outdated
Show resolved
Hide resolved
src/plugin/safe-empty-array-where-in/safe-empty-array-where-in.transformer.ts
Outdated
Show resolved
Hide resolved
src/plugin/safe-empty-array-where-in/safe-empty-array-where-in.transformer.ts
Outdated
Show resolved
Hide resolved
| await db.schema.dropTable('safeEmptyArrayPerson').ifExists().execute() | ||
| await createTableWithId(db.schema, dialect, 'safeEmptyArrayPerson') | ||
| .addColumn('firstName', 'varchar(255)') | ||
| .execute() | ||
|
|
||
| await db | ||
| .insertInto('safeEmptyArrayPerson') | ||
| .values([ | ||
| { | ||
| firstName: 'John', | ||
| }, | ||
| { | ||
| firstName: 'Mary', | ||
| }, | ||
| { | ||
| firstName: 'Tom', | ||
| }, | ||
| ]) | ||
| .execute() |
There was a problem hiding this comment.
why do we need a special table for this suite?
There was a problem hiding this comment.
whats the convention in this project? im doing this just so to avoid collisions with other tests suites if things are run in parallel
There was a problem hiding this comment.
Things are not run in parallel. Each suite creates and destroys everything.
| let result = await query.execute() | ||
|
|
||
| expect(result).to.deep.equal([]) |
There was a problem hiding this comment.
no need to execute really, testing the compiled sql is more than enough.
There was a problem hiding this comment.
i think we should execute to make sure there are no run-time errors, which was the case before.
| let result = await query.execute() | ||
|
|
||
| expect(result).to.have.length(2) | ||
| expect(result).to.deep.equal([ | ||
| { firstName: 'John' }, | ||
| { firstName: 'Mary' }, | ||
| ]) |
There was a problem hiding this comment.
no need to execute really, testing the compiled sql is more than enough.
There was a problem hiding this comment.
Could we opt to keep to test for runtime errors?
| let result = await query.execute() | ||
|
|
||
| expect(result).to.deep.equal([new DeleteResult(BigInt(0))]) |
There was a problem hiding this comment.
no need to execute really, testing the compiled sql is more than enough.
There was a problem hiding this comment.
Ah, can i leave it here since its already there? Would be good for us to know side-effects if any for results as well.
|
@igalklebanov ive addressed all of the PR comments, could i check if this is ready to be merged? |
src/plugin/safe-empty-array-where-in/safe-empty-array-where-in.plugin.ts
Outdated
Show resolved
Hide resolved
|
@austinwoon CI seems to run forever, can you check this? |
This problem seems to not be from my PR. I checked out could you check on this? |
Addresses concerns #709 with a plugin.