-
Notifications
You must be signed in to change notification settings - Fork 71
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
Node tests - transactions fix #2611
Conversation
… past 7.2.0 Signed-off-by: Ubuntu <[email protected]>
7c7b5ad
to
dfe1207
Compare
Signed-off-by: Ubuntu <[email protected]>
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.
Duplicate of #2536, please close
node/tests/SharedTests.ts
Outdated
if (!cluster.checkIfServerVersionLessThan("7.2.0")) { | ||
expect(result[0].inactive).toBeGreaterThan(0); | ||
if (cluster.checkIfServerVersionLessThan("7.2.0")) { | ||
expect(result[0].idle).toBeGreaterThan(0); // Use `idle` for Redis versions before 7.2.0 |
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.
idle
is checked 2 lines above
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.
@Yury-Fridlyand Deleted the WAIT transactions test completley.
I've seen in your PR you're running this test for versions greater than 7.0.0. but note that it's also failing for 7.2
the whole WAIT in the transactions is flaky as it seems to be relevant to how fast the engine handles it, sometimes it does succeed on 6.2 and sometimes it fails, so we can't be completely sure how it behaves
Anyhow, we agreed upon deleting the test and merging this PR since we need it ASAP
Signed-off-by: Muhammad Awawdi <[email protected]>
Signed-off-by: Muhammad Awawdi <[email protected]>
Signed-off-by: Muhammad Awawdi <[email protected]>
Signed-off-by: Muhammad Awawdi <[email protected]>
Signed-off-by: Muhammad Awawdi <[email protected]>
The change will be just removing wait from transaction, if you have disagreement, we will discuss a change base on decision. |
Overiding since we merging a specific change, the discussion is still open if you see fit.
Issue link
This Pull Request is linked to issue (URL): #2622
Checklist
Before submitting the PR make sure the following are checked: