Skip to content

use matrix to test against both SQL Server 2016 and 2008#428

Merged
xiangyushawn merged 6 commits intomicrosoft:devfrom
xiangyushawn:test-agaist-both-2008-and-2016
Dec 1, 2017
Merged

use matrix to test against both SQL Server 2016 and 2008#428
xiangyushawn merged 6 commits intomicrosoft:devfrom
xiangyushawn:test-agaist-both-2008-and-2016

Conversation

@xiangyushawn
Copy link
Copy Markdown
Contributor

@xiangyushawn xiangyushawn commented Aug 2, 2017

PR #543 fixes failures that are not caught by CIs on SQL Server 2008 because we do not have CI running against 2008.

Create this PR to test both 2008 and 2016 on Appveyor by using matrix. This will help us improve PR qualities.

This PR fails for now on SQL Server 2008 until we merge PR #543

@codecov-io
Copy link
Copy Markdown

codecov-io commented Aug 2, 2017

Codecov Report

Merging #428 into dev will decrease coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##                dev     #428     +/-   ##
===========================================
- Coverage     46.55%   46.45%   -0.1%     
+ Complexity     2214     2210      -4     
===========================================
  Files           108      108             
  Lines         25321    25321             
  Branches       4175     4175             
===========================================
- Hits          11789    11764     -25     
- Misses        11506    11538     +32     
+ Partials       2026     2019      -7
Flag Coverage Δ Complexity Δ
#JDBC41 46.29% <ø> (+0.01%) 2200 <ø> (+2) ⬆️
#JDBC42 46.35% <ø> (-0.13%) 2207 <ø> (-6)
Impacted Files Coverage Δ Complexity Δ
...m/microsoft/sqlserver/jdbc/SQLServerException.java 75.2% <0%> (-1.66%) 28% <0%> (-1%)
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 44.94% <0%> (-1.13%) 16% <0%> (-1%)
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 44.24% <0%> (-0.68%) 103% <0%> (-2%)
...c/main/java/com/microsoft/sqlserver/jdbc/Util.java 60.75% <0%> (-0.67%) 87% <0%> (-1%)
...n/java/com/microsoft/sqlserver/jdbc/Parameter.java 62.01% <0%> (-0.43%) 63% <0%> (ø)
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 62.87% <0%> (-0.37%) 0% <0%> (ø)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 53.49% <0%> (-0.24%) 0% <0%> (ø)
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java 32.73% <0%> (-0.13%) 241% <0%> (ø)
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 52.51% <0%> (-0.07%) 237% <0%> (-1%)
...m/microsoft/sqlserver/jdbc/SQLServerStatement.java 59.75% <0%> (+0.1%) 134% <0%> (+1%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e1f663...6607087. Read the comment docs.

appveyor.yml Outdated
- mssql_jdbc_test_connection_properties: jdbc:sqlserver://localhost:1433;instanceName=SQL2016;databaseName=master;username=sa;password=Password12!;

services:
- mssql2008r2sp2
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it work?
I guess you should add services also in matrix.

matrix:
  - SQL: mssql2008r2sp2
   mssql_jdbc_test_connection_properties: jdbc:sqlserver://localhost:1433;instanceName=SQL2008R2SP2;databaseName=master;username=sa;password=Password12!;
  - SQL: mssql2016
   mssql_jdbc_test_connection_properties: jdbc:sqlserver://localhost:1433;instanceName=SQL2016;databaseName=master;username=sa;password=Password12!;

I did not check this but definitely you want matrix of services too.

Ref: https://stackoverflow.com/questions/42690548/multiple-combinations-of-python-and-sql-server-on-appveyor

Also please note that all MS SQL servers use the same default port 1433. Therefore please start and stop them sequentially to avoid port conflicts. To allow all SQL Server instances to be started simultaneously, please run this script at init stage.

Ref: https://www.appveyor.com/docs/services-databases/

Copy link
Copy Markdown
Contributor Author

@xiangyushawn xiangyushawn Aug 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @nsidhaye Hope you are doing well! from my investigation, matrix does not support services. Before I create this PR, I tested it in my personal fork and it worked this way. Thank you for sharing the link, I definitely will try it! If it works I will update my PR. Thank you :)

Copy link
Copy Markdown
Contributor Author

@xiangyushawn xiangyushawn Aug 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you @nsidhaye for sharing the link! I didn't find this link when I was doing investigation. I really love the idea to specify variables in matrix and then use CMD to start services! I updated my PR. Thank you for knowledge sharing!

@AfsanehR-zz AfsanehR-zz added this to the 6.3.2 milestone Aug 28, 2017
@ulvii ulvii modified the milestones: 6.3.3, 6.3.2 Aug 31, 2017
@xiangyushawn xiangyushawn removed this from the 6.3.3 milestone Sep 21, 2017
@cheenamalhotra cheenamalhotra added this to the 6.3.6 milestone Nov 30, 2017
@xiangyushawn xiangyushawn merged commit 907e1a6 into microsoft:dev Dec 1, 2017
@xiangyushawn xiangyushawn deleted the test-agaist-both-2008-and-2016 branch December 1, 2017 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants