search

adlnet / lrs-conformance-test-suite

A NodeJS project that tests the MUST requirements of the xAPI Spec and is based on the ADL testing requirements repository. The test suite website can be found here: https://lrstest.adlnet.gov/. The adopters website can be found here: https://adopters.adlnet.gov/
https://adlnet.gov/projects/xapi/
MIT License
67 stars 42 forks source link

XAPI-00171 doesn't accept substatement activity object with objectType #259

Closed Selindek closed 1 year ago

Selindek commented 1 year ago

'should process using GET with "format" ids (XAPI-00171)'

in line 1297 of H.Communication2.1-StatementResource.js it checks the keys in the subStatement's object:

expect(Object.keys(stmt.object.object).length).to.eql(1);

But that is an activity, so the objectType key optional. The proper check should be:

expect(Object.keys(stmt.object.object).length).to.be.within(1, 2);
thomasturrell commented 1 year ago

@milt I would appreciate your thoughts on this issue.

vbhayden commented 1 year ago

Had to look over this for a second, but everything seems correct.

In this case, it's requesting a specific statement back that it'd sent earlier in the test prep. The sub-statement in question that it's checking against is: https://github.com/adlnet/lrs-conformance-test-suite/blob/LRS-2.0/test/v1_0_3/templates/statements/unicode.json

Since that particular request is using format=ids and providing a specific statementId to get the specific one it sent earlier, the LRS is expected to reply with only that one statement and to exclude all non-essential information anyway, so the expected sub-statement would still be fine -- as its objectType value of Activity is the assumed one anyway, and can be excluded.

It does seem a tad opinionated though.

thomasturrell commented 1 year ago

@Selindek I think that this issue can be closed because I believe that this test in the test suite is correct according to the xAPI specification.

It might be said that the xAPI spec should make ObjectType mandatory to avoid edge cases like these but that is a different issue.

@vbhayden thank you for looking at this, I really appreciate the time you have spent on it. The test suite is a valuable resource for vendors.