Normalized and validated driverNames of test db clients and fixed oracle test setup #2692

elhigu · 2018-07-06T21:22:36Z

Removed/converted tests referring to old oracle driver to be oracledb tests
Fixed all tests to use driverName to check which driver is used for test
Updated oracledb docker script to work again
Updated travis configuration to run oracledb tests in separate environments with npm run oracledb:test
Fixed mssql server database initialization to retry if first attempt fails because server not being ready

elhigu · 2018-07-06T22:53:14Z

Looks like actually 50 more tests are failing... will fix + update travis to run oracledb test in completely test instances from others to speedup test env building.

…cle test setup

…ran in ages

…server was not ready

elhigu · 2018-07-08T00:43:38Z

Now this is ready for review

kibertoad · 2018-07-08T06:24:26Z

.travis.yml

+  - node_js: "6"
+    env: DB="mssql mysql mysql2 postgres sqlite3" KNEX_TEST_TIMEOUT=60000
+  - node_js: "10"
+    env: TEST_ORACLEDB=true NODE_VER=10 KNEX_TEST_TIMEOUT=60000


Why split conf for same env into two lines?

To not have to run first all normal tests and then start building oracledb docker image, run source building and run all tests one more time just for oracle. Though I have been thinking of committing prebuilt base image for running oracledb tests to dockerhub so that setting up that container would be lot faster.

aa I think now I got what you meant... true.

I'll try it out a bit different way.

Either of these didn't work:

- node_js: "10" env: - DB="mssql mysql mysql2 postgres sqlite3" KNEX_TEST_TIMEOUT=60000 - TEST_ORACLEDB=true NODE_VER=10 KNEX_TEST_TIMEOUT=60000 - TEST_ORACLEDB=true NODE_VER=8 KNEX_TEST_TIMEOUT=60000 - TEST_ORACLEDB=true NODE_VER=6 KNEX_TEST_TIMEOUT=60000

matrix: include: - node_js: "10" env: DB="mssql mysql mysql2 postgres sqlite3" KNEX_TEST_TIMEOUT=60000 env: TEST_ORACLEDB=true NODE_VER=10 KNEX_TEST_TIMEOUT=60000 env: TEST_ORACLEDB=true NODE_VER=8 KNEX_TEST_TIMEOUT=60000 env: TEST_ORACLEDB=true NODE_VER=6 KNEX_TEST_TIMEOUT=60000 - node_js: "8" env: DB="mssql mysql mysql2 postgres sqlite3" KNEX_TEST_TIMEOUT=60000 - node_js: "7" env: DB="mssql mysql mysql2 postgres sqlite3" KNEX_TEST_TIMEOUT=60000 - node_js: "6" env: DB="mssql mysql mysql2 postgres sqlite3" KNEX_TEST_TIMEOUT=60000

that matrix notation works a bit different way than normal travis config, but it is necessary to be able to setup different environment variables for different node versions.

@elhigu
What about

- node_js: "10" env: DB="mssql mysql mysql2 postgres sqlite3" KNEX_TEST_TIMEOUT=60000 env: TEST_ORACLEDB=true NODE_VER=10 KNEX_TEST_TIMEOUT=60000 - node_js: "8" env: DB="mssql mysql mysql2 postgres sqlite3" KNEX_TEST_TIMEOUT=60000 env: TEST_ORACLEDB=true NODE_VER=8 KNEX_TEST_TIMEOUT=60000 - node_js: "7" env: DB="mssql mysql mysql2 postgres sqlite3" KNEX_TEST_TIMEOUT=60000 - node_js: "6" env: DB="mssql mysql mysql2 postgres sqlite3" KNEX_TEST_TIMEOUT=60000 env: TEST_ORACLEDB=true NODE_VER=6 KNEX_TEST_TIMEOUT=60000

?

It won't work either, because only the last env under each section is going to be accounted for (last key overrides earlier one with the same name in yaml)

Ah, I see :)

Also I think it is easier to read when all oracle envs are one after each other compared to sprinkling them under many sections.

kibertoad · 2018-07-08T06:27:16Z

test/integration/builder/additional.js

@@ -113,7 +113,7 @@ module.exports = function(knex) {
    });

    // TODO: This doesn't work on oracle yet.
-    if (['postgresql', 'mssql'].includes(knex.client.dialect)) {
+    if (['pg', 'mssql'].includes(knex.client.driverName)) {


Probably we should start using enum for this :)

Probably, but I won't be doing that in this PR :) Its a bit out of scope to change all of those to enums. Also now enums won't bring much help anymore, because most of the used string values are validated to be one of available driverNames.

* Kill queries after timeout for PostgreSQL * Fix cancellation connection acquiring and test. * Fix releasing connection in case query cancellation fails * Add support for native enums on Postgres (#2632) Reference https://www.postgresql.org/docs/current/static/sql-createtype.html Closes #394 Signed-off-by: Will Soto <will.soto9@gmail.com> * Removal of 'skim' (#2520) * Allow overwriting log functions (#2625) * Example build of custom log functions * Handle logger object better for transactions * Adjust test to ignore sqlite warning message * Fixed onIn with empty values array (#2513) * Drop support for strong-oracle (#2487) * Remove babel-plugin-lodash (#2634) While in theory, this may reduce the bundle size, in practice it adds a ton of overhead during startup due to the number of additional requires. Bundle size also shouldn't matter for server side modules. * Add json support to the query builder for mysql (#2635) * Add json support to the query builder for mysql refs #1036 Based on #1902 * Clarify supported version * fix wrapIdentifier not being called in postgres alter column (#2612) * fix wrapIdentifier not being called in postgres alter column * add test for wrapIdentifier call in postgres alter column * add comment regarding issue * add issue & PR #'s in comment * Remove readable-stream and safe-buffer (#2640) * Add json support to the query builder for mysql refs #1036 Based on #1902 * Clarify supported version * Use native Buffer and Stream implementations * fixes 2630 (#2642) * Timeout errors shouldn't silently ignore the passed errors, but rather reject with original error. Fixes #2582 (#2626) * chore: add Node.js 10 (#2594) * chore: add Node.js 10 * chore: trigger new build * chore: update lockfile * chore: trigger new build * fix: use npm i instead of npm ci * Fix mssql driver crashing (#2637) * Run SQL Server tests locally running SQL server in docker * WIP mssql test stuff * Patched MSSQL driver to not crash knex anymore * Removed semicolon from rollback stmt for oracle (#2564) * Remove WebSQL dialect (#2647) * Add json support to the query builder for mysql refs #1036 Based on #1902 * Clarify supported version * Use native Buffer and Stream implementations * Remove WebSQL dialect * add homepage field to package.json (#2650) * Make the stream catch errors in the query (#2638) * Make the stream catch errors in the query * Fix another case in which stream doesnt emits error * Linter stuff * Remove setTimeout in tests * Make a test not to check the MySQL error code * Fix stream error catching for MariaDB and PostgreSQL * Fix stream error catching in Oracle * Throw the error after emitting it to the stream * Throw the error without instantiating a new Error * Various fixes to mssql dialect (#2653) * Fixed float type of mssql to be float * Many tests where postgres test was not actually ran at all * Migrations to be mssql compatible Mssql driver doesn't handle if multiple queries are sent to same transaction concurrently. * Prevented mssql failing when invalid schema builder was executed by accident Instead of trying to generate sql from broken schema calls, just make exception to leak before query compiling is started * Fixed mssql trx rollback to always throw an error Also modified some connection test query to be mssql compatible * Fixed various bugs from MSSQL driver to make tests run * Fixed mssql unique index to be compatible with other dialect implementations * Enable running mssql tests on CI * Test for #2588 * Updated tests to not be dependend on tables left from previous test rans * Trying to make mssql server work on travis * Updated changelog and version * Drop mariadb support (#2681) * Dropped support for mariasql * ESLint * Fixed docs to build again * Fix knex.initialize() and adds test (#2477) * Fix knex.initialize() and adds test * Fix knex.initialize() and adds test * Added test for reinitializing pool after destroy * Fixed destroy / reinitialization test * Fixed the fix * Implement missing schema support for mssql dialect * chore: Update dependencies. Remove estraverse (#2691) * Update dependencies. Remove estraverse * Fix compatibility with new Sinon * Increase mssql timeout * Normalized and validated driverNames of test db clients and fixed oracle test setup (#2692) * Normalized and validated driverNames of test db clients and fixed oracle test setup * Fixed failed queries from old query building tests which hadn't been ran in ages * Allow selecting node version which is used to run oracledb docker tests * Improved sql tester error messages * Fixed rest of the oracledb tests * Removed invalid flag from docker-compose * Print mssql logs if initialization fails * Fixed syntax error + final tests * Added restart of failure for mssql DB initialization to try again if server was not ready * Printout always mssql logs after container is started * Fixed wait time printing after trying to connect * Use npm run oracledb:test for testing oracle in travis * Add Prettier (#2697) * Add prettier * Run files through prettier * Istanbul -> NYC (#2700) * istanbul -> nyc * Update mocha * Enforce code coverage (#2702) * Enforce code coverage * Update coverage numbers with current values * version assignment on base class, copy on tx client, fix #2705 * Update changelog * v0.15.1 * Added build step to test script. Fixes #2541 (#2712) * Revert "Added build step to test script. Fixes #2541 (#2712)" (#2714) This reverts commit 90ed8db. * Allow oracle failures for now * Fix issue with select(0) (#2711) * Fix issue with select(0). Fixes #2658 * Refactor migrator (#2695) * Refactor migrator * Fix exports * Fix ESLint * Fix migrator * Fix reference to config * Split some more * Fix table builder * Fix argument order * Merge branch 'master' of https://github.com/tgriesser/knex into feature/2690-support-multiple-migration-dirs # Conflicts: # src/migrate/index.js # test/index.js # test/unit/migrate/migrator.js * Fix #2715 (#2721) * Fix #2715, explicitly set precision in datetime & timestamp * Allow for precision in knex.fn.now, mysql time * Add test for datetime with precision * Bump changelog * 0.15.2 * Fix issues with warnPromise when migration does not return a promise. Fixes #2725 (#2730) * Add tests for multiple union arguments with callbacks and builders (#2749) * Add tests for multiple union arguments * Add some callback and raw tests to union unit tests * Compile with before update so that bindings are put in correct order (#2733) * Fix join using builder withSchema. (#2744) * Use Datetime2 for MSSQL datetime + timestamp types (#2757) * Use Datetime2 for MSSQL datetime + timestamp types Datetime2 is now the recommended column type for new date work: https://docs.microsoft.com/en-us/sql/t-sql/data-types/datetime-transact-sql?view=sql-server-2017 * Add tests for MSSQL datetime2 changes * General/document breaking change (#2774) * Add changelog entry for a breaking change * Improve entry * Allow timestamp with timezone on mssql databases (#2724) * Allow timestamp with timezone on mssql databases * Change timestamp parameter to use object instead of boolean * Update dependencies (#2772) * Feature/2690: Multiple migration directories (#2735) * Implement support for multiple migration directories * Add tests for new functionality * Fix tape tests * Pass migration directory upwards * Fix multiple directory support * Fix bugs * Rollback correctly * Fix remaining tests * Address comments * #2758: Implement fail-fast logic for dialect resolution (#2776) * Implement fail-fast logic for dialect resolution, clean-up code around. * Remove method that was deprecated long time ago * Address additional comments * Try addressing comments * Set client explicitly * Fix compatibility with older Node versions * Use columnize instead of wrap in using(). (#2713) * Use columnize instead of wrap in using(). This is an attempt to fix #2136. Also added an integration test, couldn't find any existing. * Exclude MSSQL from test as it does not support .using * Change test to not use subquery * Change test * Introduced abstraction for getting migrations (#2775) * Introduced abstraction for getting migrations This would allow a webpack migration source which is compatible with bundling. * Fixed migration validation with custom migration source * Fixed issues after rebasing on muti directory PR * Renamed parameter and fixed error message * Addressed some PR comments * Finished comment * Moved filename extension filtering into fs-migrator * Added test showing in memory custom migration source * Cleaned up how to get config * Fixed failing test * Hopefully fix tests * Fix Node.js 10 support in tests * Test for correctly releasing cancel query connection

elhigu force-pushed the fix-docker-oracle-build-and-validate-that-testsuite-driver-strings-are-correct branch 2 times, most recently from 757657f to 6aac2e9 Compare July 7, 2018 01:29

elhigu added 11 commits July 8, 2018 02:13

Normalized and validated driverNames of test db clients and fixed ora…

671eee8

…cle test setup

Fixed failed queries from old query building tests which hadn't been …

8519d51

…ran in ages

Allow selecting node version which is used to run oracledb docker tests

baba9ee

Improved sql tester error messages

c90f3f2

Fixed rest of the oracledb tests

ddd16c4

Removed invalid flag from docker-compose

e84672f

Print mssql logs if initialization fails

5bf005e

Fixed syntax error + final tests

07103fb

Added restart of failure for mssql DB initialization to try again if …

c100c8f

…server was not ready

Printout always mssql logs after container is started

bbbfd60

Fixed wait time printing after trying to connect

eb81046

elhigu force-pushed the fix-docker-oracle-build-and-validate-that-testsuite-driver-strings-are-correct branch 10 times, most recently from b7ad767 to c171051 Compare July 8, 2018 00:23

elhigu requested a review from wubzz July 8, 2018 00:43

kibertoad reviewed Jul 8, 2018

View reviewed changes

elhigu force-pushed the fix-docker-oracle-build-and-validate-that-testsuite-driver-strings-are-correct branch 3 times, most recently from dea87cc to c171051 Compare July 8, 2018 09:50

elhigu force-pushed the fix-docker-oracle-build-and-validate-that-testsuite-driver-strings-are-correct branch from c171051 to a560ff2 Compare July 8, 2018 10:00

Use npm run oracledb:test for testing oracle in travis

f80b8fd

elhigu force-pushed the fix-docker-oracle-build-and-validate-that-testsuite-driver-strings-are-correct branch from a560ff2 to f80b8fd Compare July 8, 2018 10:21

wubzz approved these changes Jul 8, 2018

View reviewed changes

elhigu merged commit cb3a85e into knex:master Jul 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normalized and validated driverNames of test db clients and fixed oracle test setup #2692

Normalized and validated driverNames of test db clients and fixed oracle test setup #2692

elhigu commented Jul 6, 2018 •

edited

Loading

elhigu commented Jul 6, 2018

elhigu commented Jul 8, 2018

kibertoad Jul 8, 2018

elhigu Jul 8, 2018

elhigu Jul 8, 2018

elhigu Jul 8, 2018

elhigu Jul 8, 2018

elhigu Jul 8, 2018

kibertoad Jul 8, 2018

elhigu Jul 8, 2018

kibertoad Jul 8, 2018

elhigu Jul 8, 2018

kibertoad Jul 8, 2018

elhigu Jul 8, 2018

Normalized and validated driverNames of test db clients and fixed oracle test setup #2692

Normalized and validated driverNames of test db clients and fixed oracle test setup #2692

Conversation

elhigu commented Jul 6, 2018 • edited Loading

elhigu commented Jul 6, 2018

elhigu commented Jul 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elhigu commented Jul 6, 2018 •

edited

Loading