FDC3 Compliance Check - First Iteration #615

dbutt-scottlogic · 2022-03-07T10:40:27Z

A simple first iteration of compliance tests that verifies the existence of desktop API methods.

Checks for FDC3/FDC3Ready event. Displays error if not found in 5 seconds.
Launches a mocha test suite that runs automatically
For each API method, calls the method. If no unexpected error, test succeeds.
See README for instructions

… to yarn (to match rest of repo)

First iteration

A few fixes

linux-foundation-easycla · 2022-03-07T10:40:31Z

The committers are authorized under a signed CLA.

✅ David Butt (8d53ad4, fd5602f, f0b36db, 6a22706, 1927025, abd0b28, 1a24d25, a51434c, 05e266a, 4d9db9b)
✅ Colin Eberhardt (8e76f5f, a07dbaf, 5ff132b)

Fix minor README error

toolbox/fdc3-compliance/src/FDC3Conformance.ts

toolbox/fdc3-compliance/src/test/fdc3.addContextListener.ts

toolbox/fdc3-compliance/src/test/fdc3.addIntentListener.ts

toolbox/fdc3-compliance/src/test/fdc3.broadcast.ts

toolbox/fdc3-compliance/src/test/fdc3.addContextListener.ts

toolbox/fdc3-compliance/src/test/fdc3.getInfo.ts

toolbox/fdc3-compliance/src/test/fdc3.joinChannel.ts

toolbox/fdc3-compliance/src/test/fdc3.open.ts

kriswest

Seems a reasonable start on the conformance project. At present, the tests only check if functions are callable and not any of their behavior (which presumably we'll see some of in a future update). I've made some comments about checking responses in these initial checks however as I think you could do some confirmation that the app(s) are set-up correctly for the rest of the tests.

A correction is needed in the fdc3.joinChannel test as indicated.

Readme could be expanded to cover more of the set-up for the test - including customisation to the manifest that will be required. Could also eventually include 1.2 manifests for the major containers (vendors to supply) - thankfully 2.0 will support multiple manifests and (if possible) some more vendor agnostic fields making them less necessary.

toolbox/fdc3-compliance/README.md

kriswest · 2022-03-16T14:17:14Z

toolbox/fdc3-compliance/README.md

+
+## Application Definition
+
+A basic FDC3 application definition, as defined in the [application directory specification](https://fdc3.finos.org/schemas/1.2/app-directory#tag/Application), is supplied in the file `appDefinition.json`. This may be useful when adding the conformance tests to an application directory.


The getting started section could be expanded upon:

add a link to the app definition file

in 1.2 the definition must be expanded on with a manifest for the container to start it

if the desktop agent doesn't connect to an appD / has other configuration steps they'll need to be taken

Then, once the application definition has been added:

Do you just launch it to start the test?

What should they then expect to happen?

etc.

We can definitely bulk up the README in this area. For this version, the tests run automatically on load, so we will make that clear.
As far as configuration goes, we are avoiding putting anything vendor-specific in the documentation, but we will make it clear that there may be additional configuration required (the current file works as-is with the existing inline manifest with Glue42).

toolbox/fdc3-compliance/src/FDC3Conformance.ts

toolbox/fdc3-compliance/src/appDefinition.json

toolbox/fdc3-compliance/src/test/fdc3.findIntent.ts

kriswest · 2022-03-16T14:44:13Z

toolbox/fdc3-compliance/src/test/fdc3.findIntent.ts

+    try {
+      await window.fdc3.findIntent("ViewChart");
+    } catch (ex) {
+      if (ex !== ResolveError.NoAppsFound) {


What about ResolveError.ResolverTimeout and ResolveError.ResolverUnavailable? They are both valid errors but probably mean the test can't continue - while NoAppsFound might be valid IF they don't have another app that handles that.

To actually test handling of intents with no apps that support them (i.e. that the correct error is returned), use some long gobbledegook intent name.

This is also a good place to check the setup (including your custom intent) was completed correctly (e.g. your custom intent was registered) and to provide good error messaging if not.

Also (unsupported) other errors - should fail out the test with a different message (as those are not a valid response).

Agreed - those are all things on the TODO list for the next iteration. For this version, anything other than the expected responses will cause a test failure.

toolbox/fdc3-compliance/src/test/fdc3.findIntentsByContext.ts

toolbox/fdc3-compliance/src/test/fdc3.getInfo.ts

toolbox/fdc3-compliance/src/test/fdc3.joinChannel.ts

Address PR feedback: - Clean up resources, i.e. unsubscribe listeners and leave channels - Use first system channel in joinChannel test - Use non-existent intent names for consistency - Remove unnecessary top-level async from tests - README improvements

ggeorgievx

Thank you for the revisit, @dbutt-scottlogic ! We are almost there.

toolbox/fdc3-compliance/src/test/fdc3.addContextListener.ts

toolbox/fdc3-compliance/src/test/fdc3.addIntentListener.ts

toolbox/fdc3-compliance/src/test/fdc3.getInfo.ts

toolbox/fdc3-compliance/src/test/fdc3.joinChannel.ts

toolbox/fdc3-compliance/src/test/fdc3.findIntent.ts

toolbox/fdc3-compliance/src/test/fdc3.findIntentsByContext.ts

toolbox/fdc3-compliance/src/test/fdc3.open.ts

toolbox/fdc3-compliance/src/test/fdc3.raiseIntent.ts

toolbox/fdc3-compliance/src/test/fdc3.raiseIntentForContext.ts

Address additional PR feedback

kriswest

First off, apologies for the delay (big product release and a vacation got in the way 😬 ). As penance I've tried to add full suggestions you can commit for proposed changes.

Overall it LGTM, however, I spotted one possible issue with error handling. Technically, the FDC3 spec says that Error objects should be thrown with the strings from the error enumerations, where the tests are assuming the strings themselves will be thrown (its annoying that javascript allows this IMHO, it perhaps should have upgraded strings to errors). E.g.

If the resolution fails, the promise will return an Error with a string from the ResolveError enumeration.
Finsemble appears to be throwing the strings, rather than Errors, which may have steered you wrong on this... The suggestions I've put in would handle it either way - however we should probably have a discussion on this (in the Standards Working Group) as it makes error handling more complex.

Also the test framework is not happy with strings being thrown on failures (Finsemble is throwing the wrong error string for fdc3.open), resulting in:

kriswest · 2022-04-21T10:19:58Z

toolbox/fdc3-compliance/README.md

+
+## Application Definition
+
+A basic FDC3 application definition, as defined in the [application directory specification](https://fdc3.finos.org/schemas/1.2/app-directory#tag/Application), is supplied in the [file](./src/appDefinition.json) `appDefinition.json`. This may be useful when adding the conformance tests to an application directory. This file may need to be updated with additional manifest information, depending on the desktop agent. Consult the host environment documentation for information on configuring new applications.


minor comment, but I'd move the appD file out of the src dir and into the root of the project (I missed the link and went looking for it)

kriswest · 2022-04-21T10:23:43Z

toolbox/fdc3-compliance/src/test/fdc3.getOrCreateChannel.ts

@@ -0,0 +1,5 @@
+describe("fdc3.getOrCreateChannel", () => {
+  it("Method is callable", async () => {
+    await window.fdc3.getOrCreateChannel("FDC3Conformance");


I assume this case will be expanded in the next version to check a Channel object was actually returned.

kriswest · 2022-04-21T10:24:08Z

toolbox/fdc3-compliance/src/test/fdc3.getSystemChannels.ts

@@ -0,0 +1,5 @@
+describe("fdc3.getSystemChannels", () => {
+  it("Method is callable", async () => {
+    await window.fdc3.getSystemChannels();


Again I expect this will be expanded in the next iteration to check that the expected response was received, particularly as the joinChannel test relies on it doing so

kriswest · 2022-04-21T10:36:04Z

toolbox/fdc3-compliance/src/test/fdc3.open.ts

+  it("Method is callable", async () => {
+    try {
+      await window.fdc3.open("ThisAppDoesNotExist");
+      throw ExpectedErrorNotThrownError;


Suggested change

throw ExpectedErrorNotThrownError;

throw new Error(ExpectedErrorNotThrownError);

kriswest · 2022-04-21T10:38:41Z

toolbox/fdc3-compliance/src/test/fdc3.findIntent.ts

+      await window.fdc3.findIntent("ThisIntentDoesNotExist");
+      throw ExpectedErrorNotThrownError;
+    } catch (ex) {
+      if (ex !== ResolveError.NoAppsFound) {


This comparison might not be correct if an Error object was thrown rather than just the string message, which is best practice in JS and is technically whats described in the API docs:

If the resolution fails, the promise will return an Error with a string from the ResolveError enumeration.

Suggested change

if (ex !== ResolveError.NoAppsFound) {

if ((ex.message ?? ex) !== ResolveError.NoAppsFound) {

kriswest · 2022-04-21T10:48:51Z

toolbox/fdc3-compliance/src/test/fdc3.findIntentsByContext.ts

+      if (ex !== ResolveError.NoAppsFound) {
+        throw ex;


This comparison might not be correct if an Error object was thrown rather than just the string message, which is best practice in JS and is technically whats described in the API docs:

If the resolution fails, the promise will return an Error with a string from the ResolveError enumeration.

Suggested change

if (ex !== ResolveError.NoAppsFound) {

throw ex;

if ((ex.message ?? ex) !== ResolveError.NoAppsFound) {

throw new Error(ExpectedErrorNotThrownError + "\nException thrown: " + (ex.message ?? ex));

kriswest · 2022-04-21T10:51:01Z

toolbox/fdc3-compliance/src/test/fdc3.open.ts

+      if (ex !== OpenError.AppNotFound) {
+        throw ex;


This comparison might not be correct if an Error object was thrown rather than just the string message, which is best practice in JS and is technically whats described in the API docs:

...it returns an Error with a string from the OpenError enumeration.

The re-thrown exception should probably be a variant of ExpectedErrorNotThrownError

Suggested change

if (ex !== OpenError.AppNotFound) {

throw ex;

if ((ex.message ?? ex) !== OpenError.AppNotFound) {

throw new Error(ExpectedErrorNotThrownError + "\nException thrown: " + (ex.message ?? ex));

kriswest · 2022-04-21T10:52:18Z

toolbox/fdc3-compliance/src/test/fdc3.raiseIntent.ts

+      if (ex !== ResolveError.NoAppsFound) {
+        throw ex;


Suggested change

if (ex !== ResolveError.NoAppsFound) {

throw ex;

if ((ex.message ?? ex) !== ResolveError.NoAppsFound) {

throw new Error(ExpectedErrorNotThrownError + "\nException thrown: " + (ex.message ?? ex));

kriswest · 2022-04-21T10:52:43Z

toolbox/fdc3-compliance/src/test/fdc3.raiseIntentForContext.ts

+
+    try {
+      await window.fdc3.raiseIntentForContext(context);
+      throw ExpectedErrorNotThrownError;


Suggested change

throw ExpectedErrorNotThrownError;

throw new Error (ExpectedErrorNotThrownError);

kriswest · 2022-04-21T10:53:17Z

toolbox/fdc3-compliance/src/test/fdc3.raiseIntentForContext.ts

+      if (ex !== ResolveError.NoAppsFound) {
+        throw ex;


Suggested change

if (ex !== ResolveError.NoAppsFound) {

throw ex;

if ((ex.message ?? ex) !== ResolveError.NoAppsFound) {

throw new Error(ExpectedErrorNotThrownError + "\nException thrown: " + (ex.message ?? ex));

kriswest · 2022-04-21T11:16:46Z

toolbox/fdc3-compliance/src/appDefinition.json

+    "publisher": "FINOS",
+    "icons": [
+      {
+        "icon": "https://fdc3.finos.org/img/fdc3-logo-2019.png"


This logo doesn't work well as an icon, i'd suggest swapping out for http://fdc3.finos.org/toolbox/fdc3-workbench/fdc3-icon-256.png
(which you can copy into this project)

Move appDefinition.json to root of project and improve error handling

linux-foundation-easycla · 2022-04-29T10:05:23Z

The committers listed above are authorized under a signed CLA.

✅ login: dbutt-scottlogic / name: David Butt (8d53ad4, fd5602f, f0b36db, 6a22706, 1927025, abd0b28, 1a24d25, a51434c, 05e266a, 4d9db9b, 4e5fa88, ac1e783, 989670d, 7f98a9c, 9ee6470, 29dd445, a74bd3a, ce05480, 2a7c08b, 7b01ea7)
✅ login: ColinEberhardt / name: Colin Eberhardt (8e76f5f, a07dbaf, 5ff132b)
✅ login: AidanZealley (c015038, b1c63e6)

ColinEberhardt · 2022-05-06T09:12:16Z

@kriswest I've added @AidanZealley to the CLA, can you please re-run the checks?

kriswest · 2022-05-06T09:26:13Z

/easycla

kriswest · 2022-05-06T09:28:39Z

/easycla

kriswest · 2022-05-06T09:39:03Z

@ColinEberhardt submitted a LF support request as I don't see the checks being re-run, pretty sure I'm doing that correctly

kriswest · 2022-05-06T14:55:33Z

@ColinEberhardt support says:

I checked EasyCLA and there is no signature on file for https://github.com/AidanZealley ID: 2430106, Aidan needs to go to "Please click here to be authorized" link in the PR comment #615 (comment) and complete the signature and acknowledgment process for EasyCLA. Once done EasyCLA should authorize the commits c015038 and b1c63e6.

When you've got that done, comment /easycla to re-run the check.

AidanZealley · 2022-05-16T09:27:06Z

/easycla

kriswest · 2022-11-10T10:34:03Z

Closing as this project has moved to it own repo and will merge back a finalized build instead

dbutt-scottlogic and others added 13 commits March 2, 2022 17:30

Initial basic checks for method existence

8d53ad4

Update readme, add error handling in case intents don't exist, switch…

fd5602f

… to yarn (to match rest of repo)

Improved formatting of missing FDC3 error and correct GitHub link

f0b36db

Fix clone instructions and add nedd newlines

6a22706

Add an application definition to aid testing

1927025

More missing newlines inserted

abd0b28

Merge pull request #1 from ScottLogic/first-iteration

1a24d25

First iteration

Merge branch 'finos:master' into master

a51434c

Documentation was missing the build step

8e76f5f

Added live-server / removed Express

a07dbaf

fixed copy step in build

5ff132b

Update port number in manifest to match default for yarn start

05e266a

Merge pull request #2 from ScottLogic/ce-tweaks

4d9db9b

A few fixes

dbutt-scottlogic and others added 3 commits March 7, 2022 15:04

Correct port number in README

4e5fa88

Merge pull request #3 from ScottLogic/ce-tweaks

ac1e783

Fix minor README error

Merge branch 'finos:master' into master

989670d

kriswest requested a review from a team March 16, 2022 13:12

kriswest added api FDC3 API Working Group project infrastructure toolbox Apps or code that isn't part of the standard but supports it use labels Mar 16, 2022

ggeorgievx requested changes Mar 16, 2022

View reviewed changes

kriswest requested changes Mar 16, 2022

View reviewed changes

dbutt-scottlogic mentioned this pull request Mar 17, 2022

Address feedback on first PR ScottLogic/FDC3#7

Open

dbutt-scottlogic and others added 3 commits March 18, 2022 09:50

Merge branch 'finos:master' into master

7f98a9c

Initial changes based on PR feedback

9ee6470

Update README based on PR feedback

29dd445

dbutt-scottlogic mentioned this pull request Mar 21, 2022

Address PR feedback ScottLogic/FDC3#9

Merged

dbutt-scottlogic and others added 2 commits March 22, 2022 14:31

Ensure joinchannel test also leaves channel afterwards

a74bd3a

dbutt-scottlogic requested review from kriswest and ggeorgievx March 23, 2022 09:29

ggeorgievx requested changes Mar 23, 2022

View reviewed changes

Additional fixes based on more PR feedback

2a7c08b

dbutt-scottlogic mentioned this pull request Mar 23, 2022

Address additional PR feedback ScottLogic/FDC3#10

Merged

Merge pull request #10 from ScottLogic/address-pr-feedback

7b01ea7

Address additional PR feedback

dbutt-scottlogic requested a review from ggeorgievx March 24, 2022 10:20

ggeorgievx approved these changes Mar 24, 2022

View reviewed changes

mindthegab mentioned this pull request Apr 16, 2022

FDC3 Software Conformance Framework #583

Closed

kriswest requested changes Apr 21, 2022

View reviewed changes

kriswest reviewed Apr 21, 2022

View reviewed changes

move appDefinition.json to root of project and improve error handling

c015038

AidanZealley mentioned this pull request Apr 28, 2022

Move appDefinition.json to root of project and improve error handling ScottLogic/FDC3#14

Merged

Merge pull request #14 from ScottLogic/pr-fixes

b1c63e6

Move appDefinition.json to root of project and improve error handling

kriswest added the conformance Conformance testing framework label Apr 29, 2022

kriswest linked an issue May 13, 2022 that may be closed by this pull request

FDC3 Software Conformance Framework #583

Closed

kriswest closed this Nov 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FDC3 Compliance Check - First Iteration #615

FDC3 Compliance Check - First Iteration #615

dbutt-scottlogic commented Mar 7, 2022

linux-foundation-easycla bot commented Mar 7, 2022 •

edited

kriswest left a comment

kriswest Mar 16, 2022

dbutt-scottlogic Mar 21, 2022

kriswest Mar 16, 2022

dbutt-scottlogic Mar 21, 2022

ggeorgievx left a comment

kriswest left a comment

kriswest Apr 21, 2022

kriswest Apr 21, 2022

kriswest Apr 21, 2022

kriswest Apr 21, 2022

kriswest Apr 21, 2022

kriswest Apr 21, 2022

kriswest Apr 21, 2022

kriswest Apr 21, 2022

kriswest Apr 21, 2022

kriswest Apr 21, 2022

kriswest Apr 21, 2022

linux-foundation-easycla bot commented Apr 29, 2022 •

edited

ColinEberhardt commented May 6, 2022

kriswest commented May 6, 2022

kriswest commented May 6, 2022

kriswest commented May 6, 2022

kriswest commented May 6, 2022

AidanZealley commented May 16, 2022

kriswest commented Nov 10, 2022


		## Application Definition

		A basic FDC3 application definition, as defined in the [application directory specification](https://fdc3.finos.org/schemas/1.2/app-directory#tag/Application), is supplied in the file `appDefinition.json`. This may be useful when adding the conformance tests to an application directory.

	throw ExpectedErrorNotThrownError;
	throw new Error(ExpectedErrorNotThrownError);

	if (ex !== ResolveError.NoAppsFound) {
	if ((ex.message ?? ex) !== ResolveError.NoAppsFound) {

	throw ExpectedErrorNotThrownError;
	throw new Error (ExpectedErrorNotThrownError);

FDC3 Compliance Check - First Iteration #615

FDC3 Compliance Check - First Iteration #615

Conversation

dbutt-scottlogic commented Mar 7, 2022

linux-foundation-easycla bot commented Mar 7, 2022 • edited

kriswest left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ggeorgievx left a comment

Choose a reason for hiding this comment

kriswest left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linux-foundation-easycla bot commented Apr 29, 2022 • edited

ColinEberhardt commented May 6, 2022

kriswest commented May 6, 2022

kriswest commented May 6, 2022

kriswest commented May 6, 2022

kriswest commented May 6, 2022

AidanZealley commented May 16, 2022

kriswest commented Nov 10, 2022

linux-foundation-easycla bot commented Mar 7, 2022 •

edited

linux-foundation-easycla bot commented Apr 29, 2022 •

edited