Is test for subscriptions wrong?

[garethsb-ghost]

The test case "should get subscription notification via websocket when adding a resource" seems wrong to me.

test\query\subscriptions.ts has:

    let testSubscription = {
      'max_update_rate_ms': 100,
      'resource_path': '/nodes',
      'params': {
        'label': 'testsubscription'
      },
      'persist': true
    };

test\resources\node.json has:

  "label": "TestNode1",

If I understand correctly, with these data, the node does not match the subscription parameters, so no notification should be received. If the testSubscription were changed to have 'params': {} or 'params': { 'label': 'TestNode1' }, then the notification should be received.

Thanks.

[garethsb-ghost]

Any comment on this?

Thinking about this further, the better option is definitely to change test\query\subscriptions.ts from:

        'label': 'testsubscription'

to:

        'label': 'TestNode1'

If the 'params' were instead made empty, any other Nodes registered with the server would be immediately sent as an initial sync grain, rather than waiting for the matching Node to be registered.

[simongysi-lawo]

Thank you for reporting this issue. We think the test case is wrong, as you pointed out. We're going to check this in detail soon (the specification seems to be a bit unclear here). Adding more test cases is planed to cover "params" in general.

[garethsb-ghost]

Thanks for the confirmation, Simon.

I agree the behaviour required by the spec for the WebSockets is quite complex. Just three examples: (1) Connections made to a subscription with a large max_update_rate_ms should in some cases not be sent data grains at the same time. (2) Non-persistent subscriptions with no connections should be expired after some time (I picked default of 12s, matching node health). (3) Supporting HTTP GET 'Upgrade' to WebSocket to create a non-persistent subscription seems unclear to me - I think an 'Upgrade' request should probably result in a redirect response in our case where websockets are being served from a different port than the Query API.

Maybe we can contribute to the test cases.