Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support extended verification results #778

Merged
merged 1 commit into from
Mar 6, 2025

Conversation

YOU54F
Copy link
Member

@YOU54F YOU54F commented Feb 12, 2025

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
Copy link
Member Author

YOU54F commented Mar 4, 2025

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
Copy link
Contributor

pahnin commented Mar 5, 2025

@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
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
Copy link
Member Author

YOU54F commented Mar 6, 2025

Thanks team :)

@YOU54F YOU54F merged commit 18f571c into master Mar 6, 2025
9 checks passed
@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