Skip to content

πŸ› Fix value uniqueness check in check-params-env.sh - extracts variable names instead of valuesΒ #1786

@coderabbitai

Description

@coderabbitai

Problem Description

During review of PR #1784, a bug was identified in the check_variables_uniq function where the value uniqueness check incorrectly extracts variable names instead of values.

Current Bug

In ci/check-params-env.sh at line 72, the script uses the same sed pattern for both variable name extraction and value extraction:

# Line 72 - Current incorrect implementation
content=$(sed '/^$/d' "${env_file_path_1}" "${env_file_path_2}" | sed 's#\(.*\)=.*#\1#' | sort)

The pattern sed 's#\(.*\)=.*#\1#' extracts everything before the '=' (the variable name), not the value after it.

Impact

  • The "values must be unique" check (when allow_value_duplicity="false") never actually inspects values
  • Duplicate values could pass undetected
  • The check gives false confidence about value uniqueness

Proposed Solution

Replace the sed pattern to correctly extract values (right-hand side of '='):

# Corrected implementation
content=$(sed -e '/^[[:space:]]*$/d' -e '/^[[:space:]]*#/d' \
    "${env_file_path_1}" "${env_file_path_2}" \
  | sed 's#^[^=]*=\(.*\)#\1#' \
  | sort)

This solution:

  • Filters out empty lines and lines with only whitespace
  • Filters out comment lines starting with '#'
  • Uses s#^[^=]*=\(.*\)#\1# to extract everything after the first '=' (the value)
  • Maintains consistency with the improved filtering approach

Acceptance Criteria

  • Value uniqueness check correctly extracts and validates values, not variable names
  • Empty lines and comments are properly filtered before extraction
  • Test with sample env files containing duplicate values to verify detection
  • Existing variable name uniqueness checks remain unaffected

Testing Approach

  1. Create test env files with known duplicate values
  2. Run the script and verify it correctly identifies duplicate values
  3. Ensure no regression in variable name uniqueness checking
  4. Test with edge cases (values containing '=', empty values, etc.)

Context

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

Status

πŸ“‹ Backlog

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions