-
Notifications
You must be signed in to change notification settings - Fork 948
Fix Spec tests 'Limbo documents stay consistent' #1994
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
ca7c617
to
672723e
Compare
672723e
to
20ff6e3
Compare
Assigning this to @mikelehen since he is back and its more of his area of expertise :) You are welcome! |
step.clientIndex === undefined || tags.indexOf(MULTI_CLIENT_TAG) !== -1, | ||
"Cannot use 'client()' to initialize a test that is not tagged with " + | ||
"'multi-client'. Did you mean to use 'spec()'?" | ||
); |
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 wonder if it'd make sense to assert the inverse as well (if this is a multi-client test, then clientIndex must be defined)? Basically:
const isMultiClientTest = tags.indexOf(MULTI_CLIENT_TAG) !== -1;
const hasClientIndex = step.clientIndex !== undefined;
assert(isMultiClientTest === hasClientIndex, "Tests can use client() if-and-only-if they are tagged with 'multi-client'.");
I don't care strongly if that seems pointless though.
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.
It's not pointless, but not necessarily worth opening an IDE for and fixing since it would not break anything. I'll leave it as an imaginary TODO :)
|
||
const docA = doc('collection/a', 1000, { matches: true }); | ||
const docADirty = doc( | ||
'collection/a', | ||
1000, | ||
{ matches: true }, | ||
{ matches: true }, |
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.
Hrm. Strange that prettier allowed an extra space before...
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.
It didn't. But the change was never pushed.
The 'Limbo documents stay consistent' used the multi-client runner instead of the spec-runner. This PR fixes the test and adds an assert to make sure that this doesn't happen again.