Skip to content

feat: support extended verification results#778

Merged
YOU54F merged 1 commit into
masterfrom
feat/support_extended_verification_results
Mar 6, 2025
Merged

feat: support extended verification results#778
YOU54F merged 1 commit into
masterfrom
feat/support_extended_verification_results

Conversation

@YOU54F

@YOU54F YOU54F commented Feb 12, 2025

Copy link
Copy Markdown
Member

Optionally the body can contain details about the Pact implementation which performed the verification, and test results, which support arbitrary json

{
  "providerApplicationVersion": "1",
  "success": true,
  "buildUrl": "http://build-url",
  "testResults": [],
  "verifiedBy": {
    "implementation": "Pact-Rust",
    "version": "1.0.0",
    "clientLanguage": {
      "testFramework": "TEST",
      "name": "TESTER",
      "version": "1.2.3"
    }
  }
}

Relates to pact-foundation/pact-go#499

Question -

Would it be better to support arbitrary fields in clientLanguage, similar to `testResults, to better support extensibility.

Screenshot 2025-02-12 at 15 58 01

@YOU54F YOU54F force-pushed the feat/support_extended_verification_results branch from 6be0735 to 1a5fdef Compare February 12, 2025 16:02
Optionally the body can contain details about the Pact implementation which performed the verification, and test results, which support arbitrary json

    {
      "providerApplicationVersion": "1",
      "success": true,
      "buildUrl": "http://build-url",
      "testResults": [],
      "verifiedBy": {
        "implementation": "Pact-Rust",
        "version": "1.0.0",
        "clientLanguage": {
          "testFramework": "TEST",
          "name": "TESTER",
          "version": "1.2.3"
        }
      }
    }
@YOU54F YOU54F force-pushed the feat/support_extended_verification_results branch from 1a5fdef to e745d3e Compare February 13, 2025 09:34
@YOU54F

YOU54F commented Mar 4, 2025

Copy link
Copy Markdown
Member Author

Hey @impurist / @pahnin

Are you both able to cast your eyes across this and suggest any improvements?

If not, I'd like to merge it, but I do have an outstanding question

Would it be better to support arbitrary fields in clientLanguage, similar to testResults, to better support extensibility, or are we okay with just using these fields explicitly

@pahnin

pahnin commented Mar 5, 2025

Copy link
Copy Markdown
Contributor

@YOU54F PR LGTM but your point does make sense, having ability store metadata is useful for future. we do have to be cautious in letting users dump whatever they want, currently testResults is text field and users can dump whatever they want

@mefellows what's your take on this, we could have something like verifierMetadata json field but only store whitelisted attributes instead of letting the users dump everything?

cc @impurist

@mefellows

Copy link
Copy Markdown
Member

Making it a key/value store might make the data less query-able. e.g. if I understand correctly, you could find all verifications for a given language or version.

This could be useful down the track and in some screens.

Having it as a key/value store provides more flexibility, but does mean that users can (and probably will) put all kind of things in there.

I think it's OK to start this way, and if we have a strong use case we can move to k/v. I believe a data migration in that direction is doable too, if we decide to.

@YOU54F

YOU54F commented Mar 6, 2025

Copy link
Copy Markdown
Member Author

Thanks team :)

@YOU54F YOU54F merged commit 18f571c into master Mar 6, 2025
@YOU54F YOU54F deleted the feat/support_extended_verification_results branch March 6, 2025 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants