GeezerVison

@lwasylow - sorry for not reviewing it yet.
I'll look at it this week.

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 1 Security Hotspot to review)
18 Code Smells

89.2% Coverage
0.0% Duplication

What's missing here? Maybe I can help

While running the feature tests locally just now, I have noticed a pitfall of using interval datatype as a time distance.
The below block fails with exceptions:

set serverout on;
begin
  ut.expect(date '2022-01-30').to_be_within(INTERVAL '1'  MONTH).of_(date '2022-01-30');
end;
/
begin
  ut.expect(date '2022-05-31').to_be_within(INTERVAL '1'  MONTH).of_(date '2022-05-31');
end;
/

begin
  ut.expect(date '2022-01-30').to_be_within(INTERVAL '1'  MONTH).of_(date '2022-01-30');
end;
Error report -
ORA-01839: date not valid for month specified
ORA-06512: at line 11
ORA-06512: at "UT3.UT_BE_WITHIN_HELPER", line 24
ORA-06512: at "UT3.UT_BE_WITHIN", line 114
ORA-06512: at "UT3.UT_EXPECTATION_BASE", line 26
ORA-06512: at "UT3.UT_BE_WITHIN", line 44
ORA-06512: at line 2
01839. 00000 -  "date not valid for month specified"
*Cause:    
*Action:

Error starting at line : 6 in command -
begin
  ut.expect(date '2022-05-31').to_be_within(INTERVAL '1'  MONTH).of_(date '2022-05-31');
end;
Error report -
ORA-01839: date not valid for month specified
ORA-06512: at line 3
ORA-06512: at "UT3.UT_BE_WITHIN_HELPER", line 24
ORA-06512: at "UT3.UT_BE_WITHIN", line 114
ORA-06512: at "UT3.UT_EXPECTATION_BASE", line 26
ORA-06512: at "UT3.UT_BE_WITHIN", line 44
ORA-06512: at line 2
01839. 00000 -  "date not valid for month specified"
*Cause:    
*Action:

This is because:
January 30 + Interval '1' month, according to Interval maths is February 30th 😞
and
May 31st + Inter

val '1' month, according to Interval maths is June 31st

So, we would really need to rethink the maths and meaning of + 1 month to be more like add_months logic which assumes that if the day of month falls beyond last day of a month, it means it should be last day of a month.
Example:

select add_months(date '2022-05-31',1) from dual;
2022-06-30 12:00:00 AM

It seems that only the interval month logic suffers from that problem.

Well. interval year can also suffer from that problem on leap years.
https://stackoverflow.com/questions/35695175/oracle-db-ora-01839-date-not-valid-for-month-specified-29-02-2016-leap-year

The stack overflow link references this spec:

http://www.inf.fu-berlin.de/lehre/SS05/19517-V/FolienEtc/sql-foundation-aug94.pdf

Table 11— “Valid values for fields in datetime items” (page 130) states:

DAY - “ Within the range 1 to 31, but further constrained by the value of MONTH and YEAR fields, according to the rules for well-formed dates in the Gregorian calendar.”

This suggests that valid Gregorian calendar days should be the result.

I’d this behaviour of INTERVAL types an Oracle bug?

Well. interval year can also suffer from that problem on leap years. https://stackoverflow.com/questions/35695175/oracle-db-ora-01839-date-not-valid-for-month-specified-29-02-2016-leap-year

I think we can work this around as we already have function to extract string from interval. I reckon we can try go this route to translate interval into add_months etc.

The stack overflow link references this spec:

http://www.inf.fu-berlin.de/lehre/SS05/19517-V/FolienEtc/sql-foundation-aug94.pdf

Table 11— “Valid values for fields in datetime items” (page 130) states:

DAY - “ Within the range 1 to 31, but further constrained by the value of MONTH and YEAR fields, according to the rules for well-formed dates in the Gregorian calendar.”

This suggests that valid Gregorian calendar days should be the result.

I’d this behaviour of INTERVAL types an Oracle bug?

More digging and Oracle state this not a bug - just the way INTERVAL works. There’s an asktom from 2005 - Mr Kyte covers it all:

https://asktom.oracle.com/pls/apex/f?p=100:11:0::::p11_question_id:1157035034361

I think I've found a solution that would still rely on internal oracle logic related to intervals.
We currently use the below pseudo-code

declare
  l_result boolean;
begin
  l_result := 
    a_actual  between (a_expected - a_distance) and (a_expected + a_distance);
end;

The above results in problems when we calculate: (a_expected -/+ a_distance) for a_distance expressed as YM Interval when the calculation would result in invalid day of month like below:

select (timestamp '2022-01-30 14:45:56.123456'  + interval '0-1' year to month) from dual;

However we can express the same intent with a different formula:

declare
  l_result boolean;
begin
  l_result := ( least( a_actual, a_expected ) - greatest(a_actual, a_expected) ) year to month <= a_distance;
end;

That way, we avoid the problems with invalid dates rather than adding/subtracting interval to date, we calculate YM distance between dates and then compare it to expected max distance.

For date datatype of expected & actual this would look like below:

declare
  l_result boolean;
begin
  l_result := ( cast(least( a_actual, a_expected ) as timestamp) - cast( greatest(a_actual, a_expected) as timestamp) ) year to month <= a_distance;
end;

I have implemented required fixes.
@lwasylow @mathewbutler - would you mind having a look at latest commit?

TODO - review readme and documentation update to assure that the matcher is well explained along with it's limitations and behaviour to make sure there is no confusion when folks will use it.

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

94.5% Coverage
0.0% Duplication

Question:
Should the to_be_within matcher allow for comparison of

• timestamp vs timestamp with time zone
• timestamp vs timestamp with local time zone
• date vs timestamp
• etc.

That is, should we relax the strict datatype check that is present for the to_equal and allow for more relaxed check?
I think now is the time for this discussion before we merge and release this new matcher.

I have fixed 2 issues with pct matcher:

1. divisor=0 on expected = 0
2. matcher worked only one way.
   ut.expect(99).to_be_within_pct(1).of_(100) was giving SUCCESS
   ut.expect(101).to_be_within_pct(1).of_(100) was giving FAILURE

With the change introduced now we will have:

begin
  ut3_develop.ut.expect(99).to_be_within_pct(1).of_(100);
  ut3_develop.ut.expect(101).to_be_within_pct(1).of_(100);
  ut3_develop.ut.expect(99).to_be_within_pct(1).of_(100);
  ut3_develop.ut.expect(101).to_be_within_pct(1).of_(100);
  ut3_develop.ut.expect(0).to_be_within_pct(1).of_(0);
  ut3_develop.ut.expect(0).to_be_within_pct(100).of_(1);
  ut3_develop.ut.expect( 1.0001 ).to_be_within_pct( -0.01 ).of_(1);
end;
/

SUCCESS
  Actual: 99 (number) was expected to be within 1 % of 100 (number)
SUCCESS
  Actual: 101 (number) was expected to be within 1 % of 100 (number)
SUCCESS
  Actual: 99 (number) was expected to be within 1 % of 100 (number)
SUCCESS
  Actual: 101 (number) was expected to be within 1 % of 100 (number)
SUCCESS
  Actual: 0 (number) was expected to be within 1 % of 0 (number)
SUCCESS
  Actual: 0 (number) was expected to be within 100 % of 1 (number)
SUCCESS
  Actual: 1.0001 (number) was expected to be within -.01 % of 1 (number)

@lwasylow

Please retry analysis of this Pull-Request directly on SonarCloud.

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

95.9% Coverage
0.0% Duplication