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

XQSuite: numeric literal in assertion annotation #4784

Open
line-o opened this issue Mar 3, 2023 · 6 comments
Open

XQSuite: numeric literal in assertion annotation #4784

line-o opened this issue Mar 3, 2023 · 6 comments

Comments

@line-o
Copy link
Member

line-o commented Mar 3, 2023

Describe the issue

A test like the one below must fail as the returned type does not match the type of the literal in the assertion annotation.

declare namespace test="http://exist-db.org/xquery/xqsuite";

declare
  %test:assertEquals(1)
function local:test-return-string () as xs:string {
  "1"
};

Annotations are severely limited in the arguments and expected value types that can be declared. Only literals are allowed and these are xs:integer (1), xs:decimal (1.0), xs:double (1e0) and xs:string ('1').
But not matching against those types further limits the options to assert returned values. At the moment they will all be cast to xs:string at some point and then compared it seems.

-- update1 --
I believe that the current type conversion for assertion annotations is a little too lax and as its turns out this is not even documented.

-- update 2 --
Any negative numeric value has to be declared as xs:string. Same goes for other types like xs:date, xs:time, xs:dateTime and all durations.

refs #4773

Expected behavior

XQSuite calls util:inspect-function internally to get the annotation values to compare the returned value against the expected value. These annotation-elements do have a type-attribute which is already used to cast values of args-annotations before the are applied as parameters to the test-function.
The same has to be done for the assertion-annotation values.

To Reproduce

xquery version "3.1";

module namespace t="http://exist-db.org/xquery/test";

declare namespace test="http://exist-db.org/xquery/xqsuite";

(: should fail, but passes :)
declare
  %test:assertEquals(1)
function local:test-return-string () as xs:string {
  "1"
};

(: should fail, but passes :)
declare
  %test:assertEquals(1)
function local:test-return-string () as xs:double {
  1e0
};

(: could fail, but passes :)
declare
  %test:assertEquals('1')
function local:test-return-string () as xs:integer {
  1
};

(: could fail, but passes :)
declare
  %test:assertEquals('1e0')
function local:test-return-string () as xs:double {
  1e0
};

(: already passes :)
declare
  %test:assertEquals('1')
function local:test-return-string () as xs:string {
  "1"
};

(: already passes :)
declare
  %test:assertEquals(1)
function local:test-return-string () as xs:integer {
  1
};

(: already passes :)
declare
  %test:assertEquals(1e0)
function local:test-return-string () as xs:double {
  1e0
};

Context (please always complete the following information)

Build: eXist-7.0.0-SNAPSHOT (current develop HEAD)
Java: 17.0.6 (Azul Systems, Inc.)
OS: Mac OS X 12.6.3 (aarch64)

Additional context

  • How is eXist-db installed? built from source
  • Any custom changes in e.g. conf.xml? none
@joewiz
Copy link
Member

joewiz commented Mar 3, 2023

Could you paste in the actual results of your XQSuite tests? It helps the reader to know what the current state is and not just the expected state.

Also, let me try to draw out the implications of what you're describing.

From the XQSuite documentation at https://exist-db.org/exist/apps/doc/xqsuite, %test:assertEquals is defined as follows:

If the function returns an atomic type, the assertion argument is cast into the same type and the two are compared using the eq operator.

Also:

XQuery annotation parameters need to be literal values, so only strings and numbers are allowed. XQSuite therefore applies type conversion to every argument and to values used in assertions.

Type conversion works for atomic types as well as XML nodes.

If automatic type conversion fails the test case will fail as well.

Is the "bug" you're describing that (a) the library does not conform to the documentation? Or are you describing scenarios that (b) the documentation doesn't explicitly touch on AND the library doesn't perform as you would expect?

Also, for reference, the relevant code for %test:assertEquals is https://github.com/eXist-db/exist/blob/develop/exist-core/src/main/resources/org/exist/xquery/lib/xqsuite/xqsuite.xql#L762-L792. It calls other functions that perform the actual equality check and the casting of annotation values into the required types.

If the bug is in the XQSuite library, can you point to where you think the problem is rooted?

@line-o
Copy link
Member Author

line-o commented Mar 3, 2023

After looking at the code and modifying it to meet my expectations I am more inclined to see this as an enhancement rather than a bug.

Because of the limitations of types in annotations it is impossible to have negative
numeric literals as values. %test:args(-1) will throw at compile time and prevent any test to run.

This means that anything but numeric, positive literals have to be expressed as strings.
"true"/"false" for booleans, "-1" for negative integer 1 and so on would still need to be cast to the expected type.

What I will propose is a change where numeric literals will never be converted to strings. This will allow at least a tiny bit more control over returned types and does work well even in mixed type sequences. This will also allow for further optimisations within the XQSuite code.

@line-o
Copy link
Member Author

line-o commented Mar 3, 2023

I was thinking of a new type of annotation %test:assertType which would allow better control over the returned type. But declaring the return type on the test function should do the trick as well.

It does not help for the above case where any positive numeric literal is automagically casted to string, if the return type of the function under test happens to be xs:string.

@line-o line-o changed the title [BUG] XQSuite must assert the type _and_ value of assertion XQSuite: numeric literal in assertion annotation Mar 3, 2023
@line-o
Copy link
Member Author

line-o commented Mar 3, 2023

@joewiz I updated this issue to reflect the new findings and also the comments above each test case describe the status-quo.

@line-o
Copy link
Member Author

line-o commented Mar 3, 2023

@joewiz I just realised that the documentation includes my additions for custom assertions even though the PR is still open #4332

Fortunately, nobody has opened an issue that custom assertions aren't working.

@line-o
Copy link
Member Author

line-o commented Mar 3, 2023

@joewiz Sidenote: the paragraph about automatic type conversion in the XQSuite documentation only mentions the %test:args annotation. It is not explicitly mentioned that this also applies to the assertion annotations and how.

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

No branches or pull requests

2 participants