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

optimize: expand unit test coverage for the [rocketmq] module. #6927

Merged
merged 16 commits into from
Oct 23, 2024

Conversation

psxjoy
Copy link
Contributor

@psxjoy psxjoy commented Oct 15, 2024

  • I have registered the PR changes.
  • I have optimized the code using seata-codestyle.xml.
  • I have checked all codes using seata-checkstyle.xml.

Ⅰ. Describe what this PR did

Add unit test cases to complete the unit testing objectives for the seata-rocketmq module.
增加单元测试用例,完成 rocketmq模块的单元测试目标

Instructions Cov. Branches Cov. Missed Cxty Missed Lines Missed Methods Missed Classes
Before 31% 8% 27 40 67 98 10 22 0 4
After 95% 83% 8 41 5 101 2 23 0 4

Ⅱ. Does this pull request fix one issue?

fix #6506

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

Not applicable.
不适用

Ⅳ. Describe how to verify it

Run mvn clean test

Ⅴ. Special notes for reviews

In the SeataMQProducer class, there's a method doSendMessageInTransaction, which calls the parent class method send(Message msg, long timeout). Since Mockito can't mock certain parent methods, the functionality has been refactored to ensure the unit test coverage exceeds 70%. The changes are as follows:

SeataMQProducer类中有个函数doSendMessageInTransaction,里面调用了父类的函数 send(Message msg, long timeout)
因为Mockit 无法模拟父类的某些方法,为了单元测试用例覆盖率达到70%以上,进行了拆分。具体的修改为:

Before

public SendResult doSendMessageInTransaction(final Message msg, long timeout, String xid, long branchId)
	throws MQClientException {
	//...
	try {
		sendResult = super.send(msg, timeout);
	} catch (Exception e) {
		throw new MQClientException("send message Exception", e);
	}
	//...
}

After

public SendResult doSendMessageInTransaction(final Message msg, long timeout, String xid, long branchId)
	throws MQClientException {
	//...
	try {
		sendResult = superSend(msg, timeout);
	} catch (Exception e) {
		throw new MQClientException("send message Exception", e);
	}
	//...
}
public SendResult superSend(Message msg, long timeout)
	throws MQClientException, MQBrokerException, RemotingException, InterruptedException {
	return super.send(msg, timeout);
}

Copy link

codecov bot commented Oct 15, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 52.69%. Comparing base (4223b85) to head (895f2dc).
Report is 1 commits behind head on 2.x.

Files with missing lines Patch % Lines
...he/seata/integration/rocketmq/SeataMQProducer.java 85.71% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #6927      +/-   ##
============================================
+ Coverage     52.53%   52.69%   +0.15%     
- Complexity     6543     6559      +16     
============================================
  Files          1122     1122              
  Lines         39858    39861       +3     
  Branches       4666     4666              
============================================
+ Hits          20940    21005      +65     
+ Misses        16924    16861      -63     
- Partials       1994     1995       +1     
Files with missing lines Coverage Δ
...he/seata/integration/rocketmq/SeataMQProducer.java 85.96% <85.71%> (+63.74%) ⬆️

... and 5 files with indirect coverage changes

Copy link
Member

@xingfudeshi xingfudeshi left a comment

Choose a reason for hiding this comment

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

Great job!

@xingfudeshi
Copy link
Member

Suggest placing this PR in the test section of the change log for better classification.

@psxjoy
Copy link
Contributor Author

psxjoy commented Oct 23, 2024

Suggest placing this PR in the test section of the change log for better classification.

I appreciate your suggestion. I have already updated the PR.

@lovepoem
Copy link
Member

LGTM

@lovepoem lovepoem merged commit dbeba61 into apache:2.x Oct 23, 2024
7 checks passed
@psxjoy psxjoy deleted the rocketmq-unit-test branch October 23, 2024 12:41
@slievrly slievrly added this to the 2.3.0 milestone Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

task: Improve the test case coverage of [rocketmq] module to 70%
4 participants