Change actor tests for requirement 9.2.0.0-* to only apply to cmi5 defined statements. #6
Conversation
…fined statements.
| alter: (st) => { | ||
| st.actor.objectType = "Group"; | ||
| } | ||
| }, | ||
| { | ||
| // statement with actor objectType set to an invalid value | ||
| // defined statement with actor objectType set to an invalid value |
There was a problem hiding this comment.
Here the actor itself is invalid, so Engine returns a http 400 on failure to deserialize the actor before even getting to cmi5 validations.
So the change to this being a defined statement isn't enough, we should also accept 400 (and indeed, I'm not sure we should accept 403, because xAPI seems to demand 400):
400 Bad Request - Indicates an error condition caused by an invalid or missing argument. The term "invalid arguments" includes malformed JSON or invalid Object structures.
And although the spec could be clearer here, where xAPI defines the "Actor" property it also indicates that the actor can be an agent, in which case the objectType is optional but must be "agent" if present, or a group, in which case the object type is required and must be "group"
https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#242-actor
There was a problem hiding this comment.
This really boils down to an argument of "does the cmi5 spec supercede the xAPI spec, or vice-versa?"
Since cmi5 says explicitly: The Actor property for all "cmi5 defined" statements MUST be of objectType "Agent"., if the cmi5 spec has priority (and one could argue it might as a superset of xAPI rules), then 403 would be relevant. Otherwise, if xAPI is the dominant spec, 400 would be correct.
To me, the right answer is "both". Since we can't easily determine which one is the superset, we make the test suite accept both.
There was a problem hiding this comment.
I've updated this and helpers.js to reflect this opinion. Back to you!
…ctor type is invalid (LTS test 005-1-invalid-au).
| alter: (st) => { | ||
| st.actor.objectType = "Group"; | ||
| } | ||
| }, | ||
| { | ||
| // statement with actor objectType set to an invalid value | ||
| // defined statement with actor objectType set to an invalid value |
There was a problem hiding this comment.
I've updated this and helpers.js to reflect this opinion. Back to you!
Addressing https://rusticisoftware.slack.com/archives/C025TQVHG/p1640101691047000
We were incorrectly testing 9.2.0.0 requirements against cmi5 allowed statements, since that section specifically only applies to cmi5 defined statements. This uses a cmi5 defined statement for the actor corruption tests.