-
Notifications
You must be signed in to change notification settings - Fork 533
RUBY-2988 Add unified test format valid-fail test for unsupported operation #2488
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
spec/runners/unified.rb
Outdated
| test.set_initial_data | ||
| lambda do | ||
| test.run | ||
| skip test.skip_reason if test.skip? |
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.
Why do we skip after running the test?
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 don't know of a way to skip inside the test, so I set the skip_reason and return from the test then have the actual skip happen right after.
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.
My question is why skipping happens after test.run? Does it mean that we execute the test regardless of presence of a skip reason, and then just do not validate results?
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.
Well no, the idea is that the reason to skip happens inside the test. So in order to actually execute the skip I exit from the test early and call the skip outside of it (since skip is not accessible inside the test)
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.
Oh, got it, thank you for the explanation!
| # HACK: other errors are possible and likely will need to | ||
| # be added here later as the tests evolve. | ||
| end.should raise_error(Mongo::Error::OperationFailure) | ||
| rescue Mongo::Error::OperationFailure, Unified::Error::UnsupportedOperation |
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.
@p-mongo can you look at this? is there an easier way to check against multiple errors?
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 would have done this the way you have done this.
| # HACK: other errors are possible and likely will need to | ||
| # be added here later as the tests evolve. | ||
| end.should raise_error(Mongo::Error::OperationFailure) | ||
| rescue Mongo::Error::OperationFailure, Unified::Error::UnsupportedOperation |
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 would have done this the way you have done this.
The idea was to follow PyMongo's lead here: mongodb/specifications#1206 (comment)