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

Fix for random JUnit failures in framework tests #762

Merged
merged 7 commits into from
Aug 17, 2018

Conversation

cheenamalhotra
Copy link
Member

Fixes random test failures due to unordered Select Queries in framework tests.

Although there are similar implementations in other tests which need to be fixed, this PR focuses on the tests that have been observed failing and are designed using framework package.

@codecov-io
Copy link

codecov-io commented Jul 30, 2018

Codecov Report

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

Impacted file tree graph

@@             Coverage Diff             @@
##                dev    #762      +/-   ##
===========================================
- Coverage     48.17%   48.1%   -0.08%     
+ Complexity     2776    2772       -4     
===========================================
  Files           116     116              
  Lines         27854   27854              
  Branches       4636    4636              
===========================================
- Hits          13418   13398      -20     
- Misses        12215   12236      +21     
+ Partials       2221    2220       -1
Flag Coverage Δ Complexity Δ
#JDBC42 47.62% <ø> (-0.08%) 2730 <ø> (-5)
#JDBC43 47.97% <ø> (-0.05%) 2767 <ø> (-2)
Impacted Files Coverage Δ Complexity Δ
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 43.76% <0%> (-1.97%) 106% <0%> (-2%)
...om/microsoft/sqlserver/jdbc/SimpleInputStream.java 51.47% <0%> (-1.48%) 11% <0%> (-1%)
...ncurrentlinkedhashmap/ConcurrentLinkedHashMap.java 38.14% <0%> (-0.87%) 42% <0%> (-1%)
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java 31.95% <0%> (-0.27%) 246% <0%> (-2%)
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 62.31% <0%> (-0.24%) 0% <0%> (ø)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 53.9% <0%> (+0.09%) 0% <0%> (ø) ⬇️
...n/java/com/microsoft/sqlserver/jdbc/DataTypes.java 79.48% <0%> (+0.19%) 5% <0%> (+1%) ⬆️
...main/java/com/microsoft/sqlserver/jdbc/Column.java 34.73% <0%> (+0.59%) 35% <0%> (+1%) ⬆️
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 46.15% <0%> (+1.09%) 16% <0%> (ø) ⬇️

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 e301555...c6246fb. Read the comment docs.

@@ -96,7 +96,11 @@ public void testISQLServerBulkRecord() throws SQLException {
if (JDBCType.BIT == sqlType.getJdbctype()) {
CurrentRow[j] = ((0 == ThreadLocalRandom.current().nextInt(2)) ? Boolean.FALSE : Boolean.TRUE);
} else {
CurrentRow[j] = sqlType.createdata();
if(j==0) {
CurrentRow[j] = i+1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we just use SQL Server's auto-increment instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Tried that, but it requires IDENTITY_INSERT to be enabled in order to make TVP and Bulk Copy work, with lots of test changes. I don't think it's worth the effort if we have to eventually enable IDENTITY_INSERT.

columnValues.add(i+1);
}

}
Copy link
Contributor

Choose a reason for hiding this comment

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

New line here.

/**
* @return Escaped "columnName"
*/
public String getEscapedColumnName() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This will fail if column name contains ] or [.

Copy link
Member Author

Choose a reason for hiding this comment

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

Our table columns in tests don't contain either, so it has no impact in our tests.

* Updates rowIndexes to all rows
* @param totalRows number of rows
*/
public void populateRowId(int totalRows) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How are the rest of the columns populated? Can't we just fetch id from the table too?

Copy link
Member Author

Choose a reason for hiding this comment

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

This function and populateValues above in the same file, populate data in 'columnValues' which are inserted iteratively in the table creation process. This is first time data insertion, so can't be fetched from anywhere.

@cheenamalhotra cheenamalhotra merged commit fad697f into microsoft:dev Aug 17, 2018
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.

6 participants