Skip to content

#481: preserve true bool PMP defaults#487

Open
GHX5T-SOL wants to merge 1 commit into
zerocracy:masterfrom
GHX5T-SOL:481
Open

#481: preserve true bool PMP defaults#487
GHX5T-SOL wants to merge 1 commit into
zerocracy:masterfrom
GHX5T-SOL:481

Conversation

@GHX5T-SOL

Copy link
Copy Markdown
Contributor

Summary:

  • Preserve boolean PMP defaults that are already coerced before final type conversion.
  • Add a regression with a PMP XML bool default of true and no factbase override.

Closes #481

Validation:

  • Pre-fix probe with in-memory PMP XML default true returned value=false.
  • Post-fix probe with the same fixture returned value=true.
  • bundle exec ruby -Itest -e "ARGV << "--no-cov"; require "./test/fbe/test_pmp"; ARGV.delete("--no-cov")" -> 9 tests, 17 assertions, 0 failures, 0 errors, 0 skips.
  • bundle exec rake test -> 284 tests, 804 assertions, 0 failures, 0 errors, 9 skips.
  • bundle exec rubocop -> 65 files inspected, no offenses detected.
  • git diff --cached --check -> passed.
  • gitleaks protect --staged --no-banner --redact --verbose -> no leaks found.

No secrets were added.

@akmhatey-ai akmhatey-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.

Checked the current head b85f0855194d6fad4c307eccf48d41e6963f71b1 against #481.

The one-line change is scoped to the failing path: an XML PMP bool default is already coerced to Ruby true, and the final bool conversion now accepts it via result.to_s == 'true'. Existing string overrides still behave as before ('true' stays true, 'false' stays false), and the new regression test covers the no-factbase-override/default-true case from the issue.

Validation I ran locally:

  • bundle install with isolated BUNDLE_PATH passed.
  • Focused PMP tests passed: 9 tests, 17 assertions, 0 failures/errors/skips.
  • bundle exec rubocop lib/fbe/pmp.rb test/fbe/test_pmp.rb --format simple passed.
  • Full bundle exec rubocop --format simple passed: 65 files, no offenses.
  • git diff --check origin/master...HEAD passed.
  • Diff-only gitleaks stdin --no-banner --redact --exit-code 1 passed with no findings.

One local limitation: full bundle exec rake test on my Windows checkout hit unrelated environment errors around SQLite temp DB cleanup permissions and /bin/bash not existing. The touched TestPmp coverage passes locally, and the hosted Ubuntu/macOS rake checks are green, so I do not see that as caused by this PR.

@0crat

0crat commented May 28, 2026

Copy link
Copy Markdown

@akmhatey-ai Hey! Nice work on that review 👍 You earned +10 points this time: started with the standard +18 base points, but had to deduct 8 since there weren't any comments posted during the review. Next time, try leaving a few comments to avoid that deduction and boost your score even higher! Your running total is now +119 - keep it up and don't forget to check your Zerocracy account too.

@yegor256

Copy link
Copy Markdown
Member

Please fix the merge conflicts so this pull request can be merged.

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.

Fbe.pmp coerces bool params twice; true XML defaults silently become false when the factbase has no entry

4 participants