Skip to content
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

[KYUUBI #5600] Fix flaky test SessionsResourceSuite #5675

Closed
wants to merge 7 commits into from

Conversation

davidyuan1223
Copy link
Contributor

@davidyuan1223 davidyuan1223 commented Nov 12, 2023

Why are the changes needed?

Because the server process is async, so when we test in ci or local, because our test operation is synchronize, but the server process is async, it will random failed, so we need to fix the problem to make sure the random test not happend.
In the SessionsResourceSuite, there may have 2 random test case failed

SessionsResourceSuite:
- open/close and count session *** FAILED ***
  1 equaled 1, but 0 did not equal 1 (SessionsResourceSuite.scala:65)
- getSessionList *** FAILED ***
  List(Map("duration" -> 0, "sessionType" -> "INTERACTIVE", "identifier" -> "96ca05ee-ecb1-49cd-a2e4-66cbf49ed343", "createTime" -> 1698822938391, "ipAddr" -> "127.0.0.1", "exception" -> "", "kyuubiInstance" -> "localhost:35415", "idleTime" -> 78, "engineId" -> "", "conf" -> Map("kyuubi.session.real.user" -> "anonymous", "testConfig" -> "testValue", "kyuubi.client.ipAddress" -> "127.0.0.1", "kyuubi.session.connection.url" -> "localhost:35415", "kyuubi.server.ipAddress" -> "localhost"), "user" -> "anonymous")) was not empty (SessionsResourceSuite.scala:104)
  1. open/close session: when we open session, the server will process in async,it may complete or error quickly, so we can not make sure the active thread count must equal to 1, we need use 1 or 0 to test
  2. list sessions: in the test case, we will execute delete sessions operation, then assert, but if we assert quickly and the session not delete complete, it will failed, so we need sleep 3 seconds to make the delete operation async complete

close #5600

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before make a pull request

Was this patch authored or co-authored using generative AI tooling?

No

@codecov-commenter
Copy link

codecov-commenter commented Nov 12, 2023

Codecov Report

Merging #5675 (6aa234c) into master (eb84e15) will decrease coverage by 0.07%.
Report is 7 commits behind head on master.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #5675      +/-   ##
============================================
- Coverage     61.47%   61.41%   -0.07%     
  Complexity       23       23              
============================================
  Files           603      603              
  Lines         35644    35664      +20     
  Branches       4875     4876       +1     
============================================
- Hits          21913    21902      -11     
- Misses        11358    11380      +22     
- Partials       2373     2382       +9     

see 11 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@davidyuan1223
Copy link
Contributor Author

@yaooqinn review?

@yaooqinn yaooqinn added this to the v1.9.0 milestone Nov 13, 2023
@davidyuan1223 davidyuan1223 removed their assignment Nov 13, 2023
@cxzl25 cxzl25 changed the title [#5600] Fix flaky test SessionsResourceSuite [KYUUBI #5600] Fix flaky test SessionsResourceSuite Nov 13, 2023
@yaooqinn
Copy link
Member

Thank you, merged to master

@yaooqinn yaooqinn closed this in b783233 Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TASK][EASY] Fix flaky test SessionsResourceSuite
3 participants