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

PHPLIB-909: Spec tests for expectedError.errorResponse assertions #989

Merged
merged 1 commit into from
Oct 17, 2022

Conversation

jmikola
Copy link
Member

@jmikola jmikola commented Oct 7, 2022

@alcaeus
Copy link
Member

alcaeus commented Oct 7, 2022

Changes for 1.12 LGTM.

Possibly irrelevant since this is a prototype for the corresponding spec change, do we plan to implement the skipped schema versions before implementing 1.12?

@jmikola
Copy link
Member Author

jmikola commented Oct 10, 2022

Possibly irrelevant since this is a prototype for the corresponding spec change, do we plan to implement the skipped schema versions before implementing 1.12?

I didn't intend on doing so, as there's quite a bit to cover and we likely can't fully the implementation without also implementing those specs (e.g. CSOT). In an ideal world, there would tests in the unified spec's valid/ and valid-failed directories to provide coverage, but I'm not sure those exist and some syntax could still require a spec implementation to properly exercise.

I'll admit that this is a frustrating side effect of how we handle schema versions, but I don't have a better alternative to suggest at the moment.

@alcaeus
Copy link
Member

alcaeus commented Oct 10, 2022

I'll admit that this is a frustrating side effect of how we handle schema versions, but I don't have a better alternative to suggest at the moment.

No worries, I think that's sensible. FWIW, we can always expand our test runner to mark certain schema version as skipped to ensure we're not marking unsupported schema versions as supported.

@jmikola jmikola force-pushed the phplib-909 branch 2 times, most recently from ffa9890 to fb6b708 Compare October 14, 2022 05:08
@jmikola jmikola marked this pull request as ready for review October 14, 2022 05:08
@jmikola jmikola merged commit 2e99cd8 into mongodb:master Oct 17, 2022
@jmikola jmikola deleted the phplib-909 branch October 17, 2022 03:58
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.

2 participants