Skip to content

Fix for random JUnit failures in framework tests#762

Merged
cheenamalhotra merged 7 commits intomicrosoft:devfrom
cheenamalhotra:bvtTestFix
Aug 17, 2018
Merged

Fix for random JUnit failures in framework tests#762
cheenamalhotra merged 7 commits intomicrosoft:devfrom
cheenamalhotra:bvtTestFix

Conversation

@cheenamalhotra
Copy link
Copy Markdown
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
Copy Markdown

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.

} else {
CurrentRow[j] = sqlType.createdata();
if(j==0) {
CurrentRow[j] = i+1;
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.

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

Copy link
Copy Markdown
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);
}

} No newline at end of file
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.

New line here.

/**
* @return Escaped "columnName"
*/
public String getEscapedColumnName() {
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.

This will fail if column name contains ] or [.

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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