search

grafana / xk6-sql

Use SQL databases from k6 tests.
http://sql.x.k6.io
Apache License 2.0
116 stars 57 forks source link

add MSSQL driver #2

Closed tingwei628 closed 3 years ago

tingwei628 commented 3 years ago

As title, I added MSSQL driver.

Thanks!

imiric commented 3 years ago

Hi @tingwei628, thanks for your contribution and terribly sorry for the extremely late response. :disappointed: I missed the GitHub notification and don't check this project often.

We'd definitely like to merge this. Would you mind fixing the merge conflicts and maybe adding a test.js for it? I'd like to improve the testing situation for this extension so that we could have integration tests for all supported RDBMSs, and run them in CI. But in the meantime moving the existing test.js for SQLite into a tests directory and adding the MSSQL variant would improve the situation. There is an official SQL Server Docker image, so it should be straightforward to test and eventually move to CI.

Thanks in advance!

tingwei628 commented 3 years ago

Hi @imiric, I've already fixed the conflict! There're two questions here:

  1. How do i add test.js ?
  2. Should I delete vendor/ ?

Thanks!

imiric commented 3 years ago

Thanks for the quick response!

  1. Similar to the existing test.js for SQLite, but using MSSQL syntax. And moving both files to a tests subdirectory. I'll add a Postgres one and maybe hook up some integration tests in CI.

  2. I removed it recently in f43fcf1.

tingwei628 commented 3 years ago

Hi @imiric, I've added tests/ in 1678c4a. Also I renamed original test.js into sqlite3_test.js and added mssql_test.js in tests/ directory. Thanks !