Skip to content

MOD-14486: Fix module args handling for odd number of words and flexible merging#245

Merged
meiravgri merged 2 commits intomasterfrom
meiravg_fix_odd_number
Apr 9, 2026
Merged

MOD-14486: Fix module args handling for odd number of words and flexible merging#245
meiravgri merged 2 commits intomasterfrom
meiravg_fix_odd_number

Conversation

@meiravgri
Copy link
Copy Markdown
Contributor

@meiravgri meiravgri commented Apr 7, 2026

Summary

Refactors fix_modulesArgs in RLTest/utils.py to support module arguments with any number of values (flags, multi-value args) without requiring semicolons.

Problem

Previously, when modulesArgs was provided as a plain string without semicolons (e.g., 'WORKERS 4 TIMEOUT 80'), the function assumed all arguments were strict key-value pairs and split them into pairs of two words. This broke for:

  • Flags (zero-value args) like FORK_GC_CLEAN_NUMERIC_EMPTY_NODES
  • Multi-value args like FLAG TIMEOUT 80

An odd number of words caused an IndexError crash.

Solution

Introduced two merge strategies based on input type:

_merge_by_words — for plain strings (no semicolons)

  • Keeps the explicit string as a single unsplit blob
  • For each default arg, extracts its key (first word) and checks if it appears as a standalone word (case-insensitive) in the explicit string
  • If the key is missing, appends the full default arg to the string
  • Prevents false substring matches (e.g., GC won't match nogc)

_merge_by_dict — for structured inputs (semicolons or lists)

  • Uses the existing dict-based merge logic
  • Each arg's key is the first word; explicit keys override defaults

fix_modulesArgs routing

  • Detects input type and routes to the appropriate merge function
  • Plain string without semicolons → _merge_by_words
  • Semicolon-separated string or list input → _merge_by_dict
  • None input → deep copy of defaults

Examples

# Plain string with defaults — word-based merge
defaults = [['FORK_GC_CLEAN_NUMERIC_EMPTY_NODES', 'TIMEOUT 90']]
fix_modulesArgs(['/mod.so'], 'workers 0 nogc FORK_GC_CLEAN_NUMERIC_EMPTY_NODES timeout 90', defaults)
# → [['workers 0 nogc FORK_GC_CLEAN_NUMERIC_EMPTY_NODES timeout 90']]
# Both default keys found in explicit string → nothing appended

# Partial override — missing defaults appended
defaults = [['WORKERS 8', 'TIMEOUT 60', 'EXTRA 1']]
fix_modulesArgs(['/mod.so'], 'WORKERS 4 TIMEOUT 80', defaults)
# → [['WORKERS 4 TIMEOUT 80 EXTRA 1']]

# Odd number of words — no crash
fix_modulesArgs(['/mod.so'], 'FLAG TIMEOUT 80')
# → [['FLAG TIMEOUT 80']]

Tests

15 unit tests in tests/unit/test_utils.py covering:

  • Plain strings with and without defaults
  • Semicolon-separated args
  • List and multi-module inputs
  • Case-insensitive matching
  • Substring collision prevention (GC vs nogc)
  • Odd word counts (no crash)
  • None input with deep copy verification

Copy link
Copy Markdown

@ofiryanai ofiryanai left a comment

Choose a reason for hiding this comment

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

Looks more comprehensive and well designed than my #244, closing mine. thanks

@meiravgri meiravgri changed the title try merging instread MOD-14486: Fix module args handling for odd number of words and flexible merging Apr 9, 2026
@meiravgri meiravgri merged commit b21c1f5 into master Apr 9, 2026
10 checks passed
@meiravgri meiravgri deleted the meiravg_fix_odd_number branch April 9, 2026 09: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