Skip to content

timing: Return FAILED when analysis failed in SegmentStoreStatisticsDp #312

Merged
bhufmann merged 1 commit into
eclipse-tracecompass:masterfrom
bhufmann:fixSegmentStoredp
Sep 2, 2025
Merged

timing: Return FAILED when analysis failed in SegmentStoreStatisticsDp #312
bhufmann merged 1 commit into
eclipse-tracecompass:masterfrom
bhufmann:fixSegmentStoredp

Conversation

@bhufmann

@bhufmann bhufmann commented Sep 2, 2025

Copy link
Copy Markdown
Contributor

What it does

timing: Return FAILED when analysis failed in SegmentStoreStatisticsDp

Fixes #302

How to test

Verify with added JUnit test case (SegmentStoreStatisticsDataProviderTest#testModuleExecutionError)

Follow-ups

N/A

Review checklist

  • As an author, I have thoroughly tested my changes and carefully followed the instructions in this template

}
}

if (!success && module.getFailureCause() != null) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What should we do in case of failure but the failure cause is null?

@bhufmann bhufmann Sep 2, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This would be not detected because waitForCompletion returns a single boolean. We only can detect failure if failure cause is not null. So, I guess it's mandatory with the current implementation.

if (monitor != null) {
module.waitForCompletion(monitor);
success = module.waitForCompletion(monitor);
if (monitor.isCanceled()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If there is failure but also monitor canceled, should the failure override the cancel? i.e. should we return cancelled only if there is no failure cause?

@bhufmann bhufmann Sep 2, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Mmh, good question. I'll check. Probably failure has priority.

Fixes eclipse-tracecompass#302

Signed-off-by: Bernd Hufmann <bernd.hufmann@ericsson.com>
@bhufmann bhufmann merged commit 095c69d into eclipse-tracecompass:master Sep 2, 2025
4 checks passed
@bhufmann bhufmann deleted the fixSegmentStoredp branch September 2, 2025 19:31
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.

Segment store provider sends back partial results with COMPLETED status

2 participants