Add conformance tests into docs #1417
Conversation
✅ Deploy Preview for fdc3 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@openfin-johans this might be a good docs PR for you to take a quick look at Johan. |
|
what's your thinking here, @kriswest? Why do we even want to do this? |
Do you mean why would we want to put the conformance test definitions into the docs? Over the last 6 months or so we've answered numerous clarifying questions to 3 teams implementing Desktop Agents. Of those at least 2, if not all 3 were referred to the conformance tests to clarify the intended behaviour in particular areas - they did not discover the tests on their own (because they are not in the documentation), but did find them useful. Two of those teams have since applied for and achieved conformance, by passing tests that implement the test definitions. Hence, the tests are useful additional detail; by adding them to the docs we make them easier to find and to scrutinize/comment on/suggest extensions and corrections. They are more formally part of the standard. We also want to start having PRs that alter the standard also address necessary changes or additions to the conformance test in the same PR - something we agreed to add to the checklist in the proposed PR template. Having the tests visible in the docs will make that easier. Finally, the tests should be better tied to specific standard versions than they are. This was a good opportunity to break from the current format which blends 1.2 and 2.0 tests (which gets preserved in the 2.0 and 2.1 versions) and to create a cleaner version for 2.2 onwards. I think that will help with your proposed rewrite of the implementation (and presumably definition of cases) in gherkin syntax for use with cucumber. |
| | A | 2. Check Metadata | Ensure that the correct app was opened | | ||
|
|
||
| - `AOpensB3`: **A** uses an `AppMetadata` or `AppIdentifier` to open B, via: | ||
| - `fdc3.open({appId: “<app B ID>”})` |
There was a problem hiding this comment.
@kriswest At the risk of being picky, there's a few places in this file where we seem to be using smart quotes rather than straight quotes - which is technically wrong for code samples, inconsistent with samples elsewhere, and usually something that happens by accident if partially authored in Word or SharePoint or something.
There was a problem hiding this comment.
Yes, those are in the current files and are not correct (this PR moved the files which is why its all showing up in the diff). Heres an example from main to compare with:
FDC3/toolbox/fdc3-conformance/Open-Tests.md
Lines 13 to 14 in d5aab3f
They could do with correcting, yes.
There was a problem hiding this comment.
@kemerava also pointed out some consistency issues with the language that also needs fixing.
This PR has only addressed adding the existing files into the website and any preamble content needed - it didn;t go as far as correcting the content (yet?)
|
|
||
| - `AOpensBWithContext3`: **A** uses an `AppMetadata` or `AppIdentifier` to open B, via: | ||
| - `fdc3.open({appId: “<app B ID>”}, <fdc3.instrument context>)` | ||
| - `AOpensBWithSpecificContext`: Perform AOpensBWithContext3 but replace **B**s call with `fdc3.addContextListener('fdc3.instrument', handler)` |
There was a problem hiding this comment.
@kriswest fdc3.addContextListener('fdc3.instrument', handler) code sample here and below is using single quotes, which is inconsistent with the use of double quotes elsewhere in the section.
|
|
||
| _These are some basic sanity tests implemented in the FDC3 Conformance Framework. It is expected that Desktop Agent testers will run these first before commencing the much more thorough tests in section 2 onwards._ | ||
|
|
||
| - `BasicCL1`: You can create a context listener by calling `fdc3.addContextListener('fdc3.contact',<handler>)`. A `Listener` object is returned and can be used to remove the listener again by calling its `unsubscribe` function. |
There was a problem hiding this comment.
@kriswest fdc3.addContextListener('fdc3.contact',<handler>) code sample here and below is using single quotes, which is inconsistent with the use of double quotes elsewhere.
| _These are some basic sanity tests implemented in the FDC3 Conformance Framework. It is expected that Desktop Agent testers will run these first before commencing the much more thorough tests in section 2 onwards._ | ||
|
|
||
| - `BasicCL1`: You can create a context listener by calling `fdc3.addContextListener('fdc3.contact',<handler>)`. A `Listener` object is returned and can be used to remove the listener again by calling its `unsubscribe` function. | ||
| - `BasicCL2`: You can create an **unfiltered** context listener by calling `fdc3.addContextListener(null,<handler>)`. A `Listener` object is returned and can be used to remove the listener again by calling its `unsubscribe` function. |
There was a problem hiding this comment.
@kriswest Also we have some spaces after the first argument in the code samples. e.g. fdc3.addContextListener(null,<handler>) instead of fdc3.addContextListener(null, <handler>), which are inconsistent with elsewhere e.g. App Channel Tests, Intents Test, and also might have line wrapping implications if not viewed in a large-width window.
resolves #1077
Adds the FDC3 conformance tests into the main documentation and docs for the 2.0 and 2.1 versions. Admonitions are included to indicate that 2.1 uses the 2.0 tests, while 2.2 tests are yet to be defined.
Pages updated:
This is a precursor to creating a version for 2.2 compliance (#1415). When creating that set, I believe we should withdraw the 1.2 tests from the docs pages (will exist as is in the version above) and focus on only the current test set. That has been applied to the 2.2 (/docs) versions already, ready for 2.2 tests to be added and labelled as new to that version.
I have not implemented the conformance test pages for 1.2 as they will require more editing to remove 2.0 tests. If that is needed in your opinion, let me know in a review.
N.B finos/FDC3-conformance-framework#264 should be merged after this to correct links in the confromance framework's readme to point to the new Overview page (replaces the README.md file)