-
-
Notifications
You must be signed in to change notification settings - Fork 166
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
[16.0][ADD] event_sale_seat_reserve #395
base: 16.0
Are you sure you want to change the base?
Conversation
4a3a521
to
29897f2
Compare
10db0da
to
c2c53f7
Compare
19336eb
to
83ddd9a
Compare
83ddd9a
to
8bc9aea
Compare
if config["test_enable"] and not self.env.context.get( | ||
"test_event_seat_reserve" | ||
): | ||
_logger.info("Test mode is enabled, skipping the reservation of the seats") | ||
return res |
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.
If you need to skip tests then your code is not properly testable. This is a big quality warning IMO
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.
Yeap, I agree..
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'm skipping test because in some standard tests, we're checking if a registration is on draft state.
But, with this module, a registration with a sale will be automatically on "reserved" state.
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.
This technique is totally legit to keep compatibility with old/other flows for the rest of the modules. It's not for this module. It's for not making the other tests (even standard ones) not to fail. I don't think there's other option.
Depends on #394