Skip to content
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

#3751: Esm interop flag #3616

Merged
merged 1 commit into from Jan 14, 2020
Merged

#3751: Esm interop flag #3616

merged 1 commit into from Jan 14, 2020

Conversation

@D10221
Copy link
Contributor

D10221 commented Jan 7, 2020

Enable esm interop behind a flag
Enable experimental, optional use
Minimum changes
Tests: migration/seeds/configuration
fix: eslint error

D10221
Enable esm interop behind a flag
Enable experimental, optional use
Minimum changes
Tests: migration/seeds/configuration
fix:  eslint error
return assertExec(
`node ${KNEX} --esm migrate:latest --knexfile=test/jake-util/knexfile-esm/knexfile.js --knexpath=../knex.js`
).then(({ stdout }) => {
assert.include(stdout, 'Batch 1 run: 1 migrations');

This comment has been minimized.

Copy link
@kibertoad

kibertoad Jan 13, 2020

Collaborator

cli-testlab 1.10.0 supports multiple assertions, so if you bump dependency, you can do

return assertExec(
    `node ${KNEX} --esm migrate:latest --knexfile=test/jake-util/knexfile-esm/knexfile.js --knexpath=../knex.js`, {
      expectedOutput: ['Batch 1 run: 1 migrations', 'one.js']
    }
  )
`node ${KNEX} --esm seed:run --knexfile=test/jake-util/knexfile-esm/knexfile.js`
).then(({ stdout }) => {
assert.include(stdout, 'Ran 1 seed files');
assert.notInclude(stdout, 'first.js');

This comment has been minimized.

Copy link
@kibertoad

kibertoad Jan 13, 2020

Collaborator

you can use notExpectedOutput and expectedOutput params for assertExec to avoid then chaining

@kibertoad

This comment has been minimized.

Copy link
Collaborator

kibertoad commented Jan 13, 2020

Thanks! There are tiny little things left, and then this can be merged :)

@kibertoad

This comment has been minimized.

Copy link
Collaborator

kibertoad commented Jan 13, 2020

Could you also please submit a PR with documentation for this on https://github.com/knex/documentation?

@D10221

This comment has been minimized.

Copy link
Contributor Author

D10221 commented Jan 14, 2020

Thanks,
Would you like me to implement the remarks, verbatim?
Or its an opportunity ? :)
Should I add it to 'test/cli/knexfile-test.spec.js' ?
Sorry, I thought the case was covered by the Jake (integration?)test
I'll check the docs

@kibertoad

This comment has been minimized.

Copy link
Collaborator

kibertoad commented Jan 14, 2020

@D10221 oh, you are right, this particular file is not using cli-testlab yet, so please ignore that comment, I'll refactor myself later.

@D10221

This comment has been minimized.

Copy link
Contributor Author

D10221 commented Jan 14, 2020

Thanks.
Opened a doc's PR
Let me know if its aligned to 'expectations' :)

@kibertoad kibertoad merged commit 6508602 into knex:master Jan 14, 2020
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kibertoad kibertoad mentioned this pull request Jan 14, 2020
1 of 3 tasks complete
@kibertoad

This comment has been minimized.

Copy link
Collaborator

kibertoad commented Jan 14, 2020

Released in 0.20.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.