-
Notifications
You must be signed in to change notification settings - Fork 17
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
RPP_Agents_Java_Spock_LastErrorLog - Task is to add a new feature to … #48
RPP_Agents_Java_Spock_LastErrorLog - Task is to add a new feature to … #48
Conversation
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.
Please also fix the tests
@@ -74,6 +75,7 @@ public class ReportPortalSpockListener extends AbstractRunListener { | |||
.orElseThrow(() -> new IllegalStateException("Unknown Spock version."))); | |||
|
|||
private final MemoizingSupplier<Launch> launch; | |||
private final Queue<String> exceptionMessageQueue = new LinkedList<>(); |
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 won't work for concurrent execution. It will produce a mess of different errors from different tems.
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.
So, in that case probably it should be a synchronized Map, isn't it? Where key is a unique identifier and value is an error message. Tell me please, what better to use as a unique identifier?
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.
Collection was changed
@@ -303,6 +305,13 @@ protected FinishTestItemRQ buildFinishTestItemRq(@Nonnull Maybe<String> itemId, | |||
FinishTestItemRQ rq = new FinishTestItemRQ(); | |||
ofNullable(status).ifPresent(s -> rq.setStatus(s.name())); | |||
rq.setEndTime(Calendar.getInstance().getTime()); | |||
if (!exceptionMessageQueue.isEmpty()) { | |||
if (StringUtils.isNotEmpty(rq.getDescription())) { |
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.
That's not going to work this way, rq.getDescription()
will be always null. You have to either save the description on the item start event, either generate it here anew.
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.
Do you mean, like this?
if (!exceptionMessageQueue.isEmpty()) {
rq.setDescription(String.format("\nError:\n%s", exceptionMessageQueue.poll()));
}
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.
Or do you mean to take description from StartTestItemRQ object and add to them error message?
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.
Second. You should not override existing description, just append the error. This was a requirement.
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.
Done
d3e0d59
to
23bae32
Compare
…RP agent for Spock. Feature - add last error log to step description.
23bae32
to
712e4d4
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #48 +/- ##
=============================================
+ Coverage 83.52% 83.90% +0.37%
- Complexity 145 147 +2
=============================================
Files 10 10
Lines 516 528 +12
Branches 44 46 +2
=============================================
+ Hits 431 443 +12
Misses 58 58
Partials 27 27 ☔ View full report in Codecov by Sentry. |
Task is to add a new feature to RP agent for Spock.
Feature - add last error log to step description. In RP there is an entity 'step' and it can have different data inside - in particular logs. It is the only step with some statistics, in fact. When during step execution an error occurs (log with Error level) it is needed to add this information to step description (a field of the 'step' entity).
There is a method for setting description: https://github.com/reportportal/commons-model/blob/develop/src/main/java/com/epam/ta/reportportal/ws/model/FinishExecutionRQ.java#L69
Take into account that description can have some information - need to append an error text, not to overwrite description value.
Description should be updated only for Step (not for Nested step - it doesn't have description).
Unit-tests must be implemented
Task description in EWORK:
https://ework.cirro.io/engineer/projects/7b2d93db-978a-46dc-9616-91a46cec7109