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

bugfix: fixed bugs in EventBus unit tests #3607

Merged
merged 2 commits into from
Apr 7, 2021
Merged

Conversation

funky-eyes
Copy link
Contributor

@funky-eyes funky-eyes commented Apr 4, 2021

Ⅰ. Describe what this PR did

单独测试DefaultCoreForEventBusTest#test时并不会出现,当与coordinator目录下的其它单元测试一并测试时,由于其中也存在需要异步提交的事务单测,导致GlobalTransactionEventSubscriber中的processGlobalTransactionEvent会进入多次相同的事务状态更改通知,导致单测无法通过
目前有2个修复方案:
1.如上面说的sleep,比较粗暴,但是能解决
2.Assertions.assertEquals强行要求了事务状态变更只能1次的通知,我觉得只要不是null就可以接受了
pr中采用方案1
理由:方案一虽然粗暴但是不会改变此单测的目的,而方案二的话就把结果改变了
image

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@funky-eyes funky-eyes added this to the 1.4.2 milestone Apr 4, 2021
@funky-eyes funky-eyes added module/test test module type: bug Category issues or prs related to bug. labels Apr 4, 2021
@codecov-io
Copy link

codecov-io commented Apr 4, 2021

Codecov Report

Merging #3607 (0c46f82) into develop (2dac3a5) will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3607      +/-   ##
=============================================
+ Coverage      52.12%   52.15%   +0.02%     
- Complexity      3502     3510       +8     
=============================================
  Files            638      638              
  Lines          21108    21108              
  Branches        2618     2613       -5     
=============================================
+ Hits           11003    11009       +6     
+ Misses          9016     9014       -2     
+ Partials        1089     1085       -4     
Impacted Files Coverage Δ Complexity Δ
...tasource/sql/struct/cache/MysqlTableMetaCache.java 82.35% <0.00%> (-1.18%) 10.00% <0.00%> (-1.00%)
...rage/redis/store/RedisTransactionStoreManager.java 64.38% <0.00%> (+0.45%) 38.00% <0.00%> (+1.00%)
...torage/file/store/FileTransactionStoreManager.java 57.69% <0.00%> (+0.64%) 29.00% <0.00%> (+1.00%)
...in/java/io/seata/server/session/SessionHolder.java 56.34% <0.00%> (+3.17%) 34.00% <0.00%> (+4.00%)

Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@l81893521 l81893521 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/test test module type: bug Category issues or prs related to bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants