-
Notifications
You must be signed in to change notification settings - Fork 375
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
Made some tests more robusts across platforms #1092
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #1092 +/- ##
=======================================
Coverage 77.19% 77.19%
=======================================
Files 25 25
Lines 7440 7440
=======================================
Hits 5743 5743
Misses 1697 1697 |
Testing decimal(18,5): | ||
****Encrypted default type is compatible with encrypted decimal(18,5)**** | ||
Testing decimal\(18,5\): | ||
-----Encrypted default type is compatible with encrypted decimal\(18,5\)----- | ||
c_det: -9223372036854.80000 | ||
c_rand: 9223372036854.80000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed backslashes here?
[6]=> | ||
string(18) "222.22200000000001" | ||
string(%d) "222.222%S" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify, this change in string length is because the value returned may have a different number of digits on different platforms, is that right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
@@ -425,7 +425,7 @@ string(10) "STRINGCOL2" | |||
string(23) "2000-11-11 11:11:11.223" | |||
string(10) "STRINGCOL2" | |||
string(10) "STRINGCOL2" | |||
string(18) "222.22200000000001" | |||
string(%d) "222.222%S" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are other floating point values in this test's output, are you sure the other values are the same across different platforms?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far, yes
global $daasMode, $localeDisabled; | ||
return ($daasMode || $localeDisabled); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't use isDaasMode anymore now right? We can remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still use it in specific tests for Azure
\[4\] => 1 | ||
\[5\] => 9999999999999999999999999999999999999 | ||
\[6\] => 922337203685477.5807 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the backslashes are needed here too (and in a few other places) to match a '.'
@@ -38,7 +41,7 @@ do { | |||
echo "Connection failed at $elapsed secs. Leeway is $leeway sec but the difference is $diff\n"; | |||
} else { | |||
// The test will fail but this helps us decide if this test should be redesigned | |||
echo "$numAttempts\t"; | |||
echo "$numAttempts: $diff\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could label the output to make it immediately obvious what happens if the test fails in a full test run, otherwise we just get two numbers. Something like echo "Attempts: $numAttempts, Time difference: $diff\n"
.
No description provided.