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

Test suite sends (optionally) invalid interactions, expects `200` #127

Closed
bscSCORM opened this issue Jan 25, 2017 · 4 comments

Comments

@bscSCORM
Copy link

commented Jan 25, 2017

It looks like the test is sending matching_no_target.json ( which is a matching activity that (incorrectly) does not include a target) in a statement and expects the LRS to respond with 200

statement activity "source" uses matching is an array of interaction components
Expected response status code to be 200 got 400

Since:

An LRS, upon consuming a valid interactionType, MAY validate the remaining properties as specified for Interaction Activities and MAY return 400 Bad Request if the remaining properties are not valid for the Interaction Activity.

The test needs to allow either 200 or 400 when sending statements that include a valid interactionType, but do not strictly adhere to the remaining documentation for that interactionType. Alternatively, the test could avoid sending such statements.

Note: the details for each interactionType are a little under-specified, which should give LRSs room to validate strictly or not if desired, based on the MAY above, and deference should be given to LRSs validating to ensure the interactions "make sense". So in this example, "target" is in a table listing "supported properties", it's not clear that "target" is required, but it's also not listed as optional. And the interaction activity doesn't make sense without it.

@bscSCORM

This comment has been minimized.

Copy link
Author

commented Jan 26, 2017

Note: I saw this issue as of 2efa228 (not suggesting it was caused by this commit, just noting the latest commit)

@ljwolford

This comment has been minimized.

Copy link
Contributor

commented Feb 8, 2017

create list of valid interaction activities to be included in tests for community input. once it gets the ok, include them.

@ljwolford

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2017

@bscSCORM - We're going to rip out all of those tests. There are other tests that are already sending statements with valid interaction activities that are copied from the spec. I also think any requirements written to test any interactions should be stricken from the requirements doc.

I found tests following this req: An Activity Definition uses the "interactionType" property if correctResponsesPattern is present. An LRS rejects a statement with 400 Bad Request if a correctResponsePattern is present and interactionType is not.

I couldn't find anything conclusive in the spec for that req so I will be removing those as well unless anyone finds evidence not to.

@garemoko

This comment has been minimized.

Copy link

commented Feb 10, 2017

@ljwolford see here: https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#details-10

correctResponsesPattern is not valid if used outside of Interaction Activities and interactionType is required for Interaction Activities.

See also https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#requirements-4

"Interaction Activities MUST have a valid interactionType." but "Interaction Activities SHOULD have the Activity type http://adlnet.gov/expapi/activities/cmi.interaction". So having a valid interactionType is the only thing that identifies something as being an Interaction Activity. This means that:

  • If it has a valid interaction type, then it MAY be validated based on the requirements of Interaction Activities.
  • If it has no or an invalid interaction type then it MUST be validated based on the requirements of non-Interaction Activities (including the requirement not to include the interactionType and other Interaction Activity specific properties).
@ljwolford ljwolford referenced this issue Feb 10, 2017
@cr8onski cr8onski closed this Mar 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.