Skip to content

MOB-53647 Aggregator: propagate extend-aggregation to nested underlings - #2022

Merged
henrychv merged 1 commit into
masterfrom
MOB-53647_eft_bug_fix_taurus
Sep 3, 2026
Merged

MOB-53647 Aggregator: propagate extend-aggregation to nested underlings#2022
henrychv merged 1 commit into
masterfrom
MOB-53647_eft_bug_fix_taurus

Conversation

@devership16

@devership16 devership16 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes ValueError: substring not found crash in __extend_reported_data when extend-aggregation: true is enabled for BBT (selenium) tests.

Root Cause

ApiritifLoadReader extends ConsolidatingAggregator and is added as an underling of the outer ConsolidatingAggregator. The chain:

Outer ConsolidatingAggregator (engine module, startup() called)
  └── ApiritifLoadReader (inner ConsolidatingAggregator, startup() NOT called)
       └── JTLReader (reads CSV samples)

The outer's startup() propagates _redundant_aggregation=True to the inner via set_aggregation(). But the inner's startup() is never called by the engine (it's just an underling, not a module). So the inner never propagates the flag to its underlings (JTLReader).

Without the flag, JTLReader's __add_sample skips get_mixed_label(), producing raw labels like "Navigate to site" without the required -state suffix. When the outer's converter runs __extend_reported_data, key.rindex('-') raises ValueError.

Fix

set_aggregation() now recursively propagates to nested underlings:

def set_aggregation(self, aggregation):
    self._redundant_aggregation = aggregation
    for underling in getattr(self, 'underlings', []):
        underling.set_aggregation(aggregation)

No risk of infinite loop — the underling tree is a DAG (directed acyclic graph). Leaf nodes like JTLReader have underlings = [], so recursion terminates. Underlings never reference their parent, so no cycles are possible.

Test plan

  • test_nested_aggregator_propagates_extend_aggregation — verifies flag propagation and mixed labels
  • All 19 existing consolidating aggregator tests pass

Companion fix: Blazemeter/a.blazemeter.com#8329 (allows BBT executors in isTransactionFilterAllowed)

🤖 Generated with Claude Code

Root cause: ApiritifLoadReader extends ConsolidatingAggregator and is
added as an underling of the outer ConsolidatingAggregator. The outer's
startup() calls set_aggregation(True) on the inner, but the inner's own
startup() is never called by the engine — so the inner's underlings
(JTLReader) never received _redundant_aggregation=True.

Without the flag, JTLReader's __add_sample skips get_mixed_label(),
producing raw labels (e.g. "Navigate to site") without the required
"-state" suffix. When the outer's converter runs __extend_reported_data,
key.rindex('-') raises ValueError.

Fix: set_aggregation() now recursively propagates to nested underlings.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI left a comment

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.

🟢 Approval recommended

The change is narrowly scoped, matches the stated root cause, and is covered by a targeted regression test.

Pull request overview

This PR fixes a crash in Taurus’ ConsolidatingAggregator when extend-aggregation: true is used in nested aggregation setups (e.g., outer engine aggregator → ApiritifLoadReaderJTLReader). The core change ensures the “redundant aggregation” flag is propagated recursively so nested readers generate mixed labels with the expected -state suffix, preventing ValueError in __extend_reported_data.

Changes:

  • Recursively propagate set_aggregation() to nested underlings so inner aggregators forward the flag to their own readers.
  • Add a unit test that constructs an outer→inner→reader chain and asserts flag propagation and -state suffixed labels.
File summaries
File Description
bzt/modules/aggregator.py Makes ResultsProvider.set_aggregation() propagate the flag through nested underlings, fixing the crash scenario.
tests/unit/modules/test_consolidatingAggregator.py Adds regression coverage for nested-aggregator propagation and mixed-label generation.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.19%. Comparing base (806cb89) to head (ef3dce3).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2022      +/-   ##
==========================================
+ Coverage   88.19%   88.19%   +0.01%     
==========================================
  Files          75       75              
  Lines       21162    21164       +2     
==========================================
+ Hits        18661    18663       +2     
  Misses       2501     2501              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@henrychv
henrychv merged commit c9c3d17 into master Sep 3, 2026
4 checks passed
@henrychv
henrychv deleted the MOB-53647_eft_bug_fix_taurus branch September 3, 2026 06:22
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.

3 participants