Conformance additions for 2.2 #1426
Conversation
✅ Deploy Preview for fdc3 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
...as this PR, #1426, does that
There was a problem hiding this comment.
Thanks @kriswest!
Another general comment/question: considering that there is also .Net (and any other future language bindings), and as we are introducing it effectively with version 2.2, should this be somehow called out at the very least in the docs here?
docs/api/conformance/Basic-Tests.md
Outdated
| | App | Step | Description | | ||
| |-----|-----------------|----------------------------------------------------------| | ||
| | A | `getAgent` | A calls `getAgent` and waits for the promise to resolve to a `DesktopAgent` instance. | | ||
| | A | `getInfo` | A can call the `getInfo()` method on the `DesktopAgent` instance to get the `ImplementationMetadata` object. <br /> Check that fdc3Version is set to 2.2. <br />Check that provider and providerVersion are populated. | |
There was a problem hiding this comment.
Here we are specifically requesting the provider and its version and in the basic test below (BasicGI1) we are just calling out "provider details", should we either update the former or the latter just to be either more specific or more vague? (I would prefer to be more specific, but not sure if the test actually checks for the version in the latter case and if that is intentional.
There was a problem hiding this comment.
Also similarly just like a general comment, a lot of these rest definitions are written in different styles across all of them, like here it is "A can" in other files it was "should", and sometimes it is just described in present tense. That is a scope creep for this PR but maybe others agree that it should be more standardized, we can create a followup issue?
There was a problem hiding this comment.
Yes the definitions are less than consistent and do need a clean-up. @robmoffat is keen to re-write all these cases and their implementation in gherkin syntax (although it's not clear that he will have time to do so next year - so that may be on the maintainers to do), which would be a good time to standardize them.
I would happily support and issue to fix the language (without changing meaning) and maybe able to collaborate on implementing that fix. It is out of scope for this PR however - but could be a good thing to do within 2.2.
There was a problem hiding this comment.
Just in the basic tests we say all of 'You can', 'An application can', 'The application should' with no variance in meaning - that is indeed horribly inconsistent!
There was a problem hiding this comment.
@kemerava I've tried to make the language in the basic tests more consistent (haven't touched the others). I've also made BasicGI1 more specific about the FDC3 version (it should match the version being tested against for conformance).
There was a problem hiding this comment.
(those changes were made in #1417
There is copy on the compliance page (https://deploy-preview-1426--fdc3.netlify.app/docs/next/fdc3-compliance#conformance-testing) saying that these tests are implemented for Javascript/Typescript by the conformance framework - but not explicitly noted that they have not been implemented for .NET. The test definitions are separate from their implementation and I think (all?) will still be valid for .NET without changes (although a review would be needed to be sure). |
|
/netlify |
|
@robmoffat or @TheJuanAndOnly99 this PR's preview is out of date. I can't tell why netlify is not re-running it after further commits to files in /docs (which is moving to /website/docs/ in a later PR). Could you take a look please? |
Co-authored-by: kemerava <52539182+kemerava@users.noreply.github.com>
|
@kriswest I've added some a few comments for some minor formatting / consistency changes. I believe the earlier changes you made look good though. |
|
Many thanks @novavi, just one approving @finos/fdc3-maintainers review to go on this one! |
website/versioned_docs/version-2.0/api/conformance/Intents-Tests.md
Outdated
Show resolved
Hide resolved
website/versioned_docs/version-2.0/api/conformance/Intents-Tests.md
Outdated
Show resolved
Hide resolved
website/versioned_docs/version-2.0/api/conformance/Intents-Tests.md
Outdated
Show resolved
Hide resolved
All API calls that used app name directly were deprecated in 2.0, e.g. https://fdc3.finos.org/docs/next/api/ref/DesktopAgent#raiseintent-deprecated as the name is potentially non-unique within even a single appD. They will still exist in the API definitions until an FDC3 3.0, however, conformance testing is not required (as deprecated API calls are optional - although they could be tested if present, however that may prove complex due to the function signature overloading). The conformance test docs for 2.0 and 2.1 still include definitions for the 1.2 conformance tests (as a single set of docs served for them all). With this PR I've removed them from the 2.2 version (i.e. the files with paths starting _docs/api/conformance). Hence, there should be no tests in those files with an AppIdentifier that contains a name - I believe that is the case but would appreciate confirmation via review (if you're up for checking - I just did but two sets of eyes are better than one). |
|
@kemerava apologies I may have misinterpreted your comment (as app name vs app Id), where you may have been referring to the format of the placeholder... There is some inconsistency in the form used. I'll leave it to you to confirm which you meant (or both) and to decide whether you want to have a crack at standardising. If you do I might suggest we swap roles and have you update the branch, close this PR, open a new one and I'll review. The branch protection rule will prevent whoever had the last push on the branch from approving the PR - but if thats you then I can approve. Up to you! |
…ts.md Co-authored-by: kemerava <52539182+kemerava@users.noreply.github.com>
…ts.md Co-authored-by: kemerava <52539182+kemerava@users.noreply.github.com>
…ts.md Co-authored-by: kemerava <52539182+kemerava@users.noreply.github.com>
|
|
kriswest
changed the title
resolves #1415
supersedes #1271
Adds conformance tests for FDC3 2.2.
Incorporates changes from/follows on from #1417.
Review deep links: