search

finos / FDC3-conformance-framework

A framework for testing whether desktop containers implement the FDC3 standard
Apache License 2.0
14 stars 15 forks source link

Refactor 1.2 tests #128

Closed ksgeorgieva closed 2 years ago

ksgeorgieva commented 2 years ago

resolves #127

Signed-off-by: Kalina Georgieva kalina.georgieva@tick42.com

linux-foundation-easycla[bot] commented 2 years ago

CLA Missing ID CLA Not Signed

kriswest commented 2 years ago

@Rob Moffat @.***> I've not reviewed this yet, but I note it would change the test definition for both 1.2 and 2.0 and will need to replicated in your PRs for those.

K

On Wed, 23 Nov 2022, 17:31 linux-foundation-easycla[bot], < @.***> wrote:

[image: CLA Missing ID] https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/13399143/490216957/128/#/?version=2 [image: CLA Not Signed] https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/13399143/490216957/128/#/?version=2

— Reply to this email directly, view it on GitHub https://github.com/finos/FDC3-conformance-framework/pull/128#issuecomment-1325429952, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAM7PBBYTYRJCLV4C2MOUL3WJZIGPANCNFSM6AAAAAASJGXWZY . You are receiving this because your review was requested.Message ID: @.***>

robmoffat commented 2 years ago

@kriswest I presume you mean that the context type names have changed from say fdc3.instrument to fdc3.instrument.somerandomid everywhere?

I'm happy to update the test definitions for this - desktop agent code shouldn't be doing anything special for a given context name, anyway, so perhaps using more random IDs is better?

The channel tests are horribly divergent between 1.2 and 2.0 at the moment. I've just completed a gap analysis in the https://docs.google.com/spreadsheets/d/10gjfyjh1mgkowgj70GDCzKWjFdJ0Uk_uOypKTUFIDAI/edit#gid=820919614 spreadsheet and I'm going to try and make both the code and the definitions match across both.

robmoffat commented 2 years ago

@kriswest have you tried this change out against Finsemble? I note that it is currently passing all the channel tests, so it would be good to confirm this still works for you

Joe-Dunleavy commented 2 years ago

@kriswest I presume you mean that the context type names have changed from say fdc3.instrument to fdc3.instrument.somerandomid everywhere?

I'm happy to update the test definitions for this - desktop agent code shouldn't be doing anything special for a given context name, anyway, so perhaps using more random IDs is better?

The channel tests are horribly divergent between 1.2 and 2.0 at the moment. I've just completed a gap analysis in the https://docs.google.com/spreadsheets/d/10gjfyjh1mgkowgj70GDCzKWjFdJ0Uk_uOypKTUFIDAI/edit#gid=820919614 spreadsheet and I'm going to try and make both the code and the definitions match across both.

@robmoffat (BasicJC1) Can join channel and broadcast has been changed to (BasicJC1) Can join channel and (BasicJC2) Can join the correct system channel ha been added. Also, (BasicJC3) Receive NoChannelFound error when joining a non-existing channel has been added.

robmoffat commented 2 years ago

OK, I just tried running this against Finsemble, and there are errors. I think we should figure out why there is a regression before we merge this.

robmoffat commented 2 years ago

HI @ksgeorgieva,

Another issue! Your commit isn't associated with an email address, so EasyCLA is borking. (Possibly a mismatch on your local git config)

https://community.finos.org/docs/development-infrastructure/code-validation/intro#email-validation

Also, I am in the middle of refactoring these tests to reduce repetition so we're going to be in merge hell.

How about in our meeting tomorrow we try and resolve this?

cheers, Rob