-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Add initial conformance test #48
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
Conversation
f907b13 to
251a447
Compare
SamMorrowDrums
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's great work to get started. I think lettings these evolve and not standing in the way is the best way to get this going, so this is a bit of a LGTM review, but I did glance over all the lines 😅
|
Well, the test currently fails so I don't want to merge it but thanks 😬 I'll probably put these behind a build tag. |
Yeah I figured you'd fix it first, but whether you do that now, or in multiple PRs to me is secondary. |
dbe5bd1 to
58b3a27
Compare
58b3a27 to
7a8e2db
Compare
| } | ||
| } | ||
|
|
||
| func diffNonNilFields(a, b interface{}, path string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't look too closely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avert your eyes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is call of Cthulhu.
Description
Given you have both
github/github-mcp-serverandmcp/githubimages available locally.Requires
-tags conformanceto avoid pollutinggo test ./...with failures due to missing deps.A failure looks like:
Notes
Lots of quality of life improvements to be had, lots of shortcuts taken, lots lots lots.
I tried to use testcontainers first to get some lifecycle management out of the box but it turns out they don't provide any way to get a handle to stdin.
I know the
mark3labsmodule provides types for us, and probably we'll end up using them, but I was exploring the schema ground up as I was writing this, so it was exploratory for me too.