-
Notifications
You must be signed in to change notification settings - Fork 183
RUST-697 / RUST-701 Add sessions and transactions to unified test runner #376
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
6706b47
to
91d2528
Compare
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 looks great! I just have a few small suggestions.
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.
looks good! tagging in the rest of the team for review
#[derive(Clone, Debug)] | ||
pub struct SessionEntity { | ||
pub lsid: Document, | ||
pub client_session: Option<Box<ClientSession>>, |
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 not sure why the ClientSession is boxed here?
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.
Clippy errors without the box, since the struct is too large otherwise.
Should I suppress the linter error instead?
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 in that case its probably just fine to keep the Box
then 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.
Changes look great to me, just echoing @abr-egn's question about the boxed session.
As above. Implemented a new unified test runner entity which wraps on a
ClientSession
and on its LSID, so that when the session is dropped, the LSID persists in the entity map to cross-reference with expected events.Currently skipping one test in
poc-transactions
because it relies on index management, which isn't implemented AFAIK.