Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sporadic test failure #204

Merged
merged 1 commit into from
Mar 20, 2023
Merged

Conversation

Fryguy
Copy link
Member

@Fryguy Fryguy commented Mar 20, 2023

If the cli_spec.rb's set_replication specs ran on macOS before database_replication_standby_spec.rb's register_standby_server specs, the former would create a $CHILD_STATUS var with a failing result. Then when the register_standby_server spec would run it stubs the forking call, but doesn't stub $CHILD_STATUS, resulting in a call to $CHILD_STATUS.success? which would return false.

While we could change the test to also stub $CHILD_STATUS.success? to return true, this only works on Ruby 2.7. On Ruby 3.0 this fails with a modify frozen object error.

Instead, what we can do is use Process.wait2 instead of Process.wait+$CHILD_STATUS.success? This way, we can stub the status object as well.

@agrare Please review.

The following is an example seed that failed for me:

SPEC_OPTS="--seed 4884" bundle exec rspec spec/database_replication_standby_spec.rb:195 spec/cli_spec.rb:702

If the cli_spec.rb's set_replication specs ran on macOS before
database_replication_standby_spec.rb's register_standby_server specs,
the former would create a $CHILD_STATUS var with a failing result. Then
when the register_standby_server spec would run it stubs the forking
call, but doesn't stub $CHILD_STATUS, resulting in a call to
$CHILD_STATUS.success? which would return false.

While we could change the test to also stub $CHILD_STATUS.success? to
return true, this only works on Ruby 2.7. On Ruby 3.0 this fails with a
modify frozen object error.

Instead, what we can do is use Process.wait2 instead of
Process.wait+$CHILD_STATUS.success? This way, we can stub the status
object as well.
@Fryguy
Copy link
Member Author

Fryguy commented Mar 20, 2023

Note that running the specs with 3.0 produces a lot of errors, but they are unrelated to this specific error.

@agrare
Copy link
Member

agrare commented Mar 20, 2023

$ SPEC_OPTS="--seed 4884" bundle exec rspec spec/database_replication_standby_spec.rb:195 spec/cli_spec.rb:702
Run options: include {:locations=>{"./spec/database_replication_standby_spec.rb"=>[195], "./spec/cli_spec.rb"=>[702]}}

Randomized with seed 4884

ManageIQ::ApplianceConsole::Cli
  #set_replication
Configuring Server as Primary
    should configure DB as primary when the required arguments are specified
Configuring Server as Primary
    should configure primary replication with a fixed database name and user when specified in the flags
Configuring Server as Standby
    should configure DDBB replication as standby when the required parameters are specified
Configuring Server as Standby
    should configure repmgrd when auto-failover flag is set

ManageIQ::ApplianceConsole::DatabaseReplicationStandby
  create standby server
    #register_standby_server
      Succeed when REGISTER_CMD succeeds

Finished in 0.01988 seconds (files took 0.36837 seconds to load)
5 examples, 0 failures

Randomized with seed 4884

@miq-bot
Copy link
Member

miq-bot commented Mar 20, 2023

Checked commit Fryguy@2e404e7 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
3 files checked, 2 offenses detected

lib/manageiq/appliance_console/database_replication.rb

  • ⚠️ - Line 154, Col 7 - Lint/UselessAssignment - Useless assignment to variable - pid. Use _ or _pid as a variable name to indicate that it won't be used.

spec/cli_spec.rb

  • ❗ - Line 707, Col 81 - Performance/StringInclude - Use String#include? instead of a regex match with literal-only pattern.

@agrare agrare merged commit f43972a into ManageIQ:master Mar 20, 2023
@Fryguy Fryguy deleted the fix_sporadic_test_failure branch March 20, 2023 21:09
jrafanie pushed a commit that referenced this pull request Jun 30, 2023
Fix sporadic test failure

(cherry picked from commit f43972a)
Fryguy added a commit that referenced this pull request Feb 7, 2024
Fixed
- Fix sporadic test failure [#204]
- Remove MIQ specific gem source [#209]
- Double escape @ in realm to avoid shell interpretation [#211]
- Move gem name loader to proper namespaced location [#208]
- Separate kerberos from service principal name and use correctly [#215]
- Add manageiq user to allowed_uids for sssd [#220]
- Remove warning about using pg_dump [#221]
- Fix specs where AwesomeSpawn private interface changed [#224]
- Change the Name of the CA from something to ApplianceCA [#228]
- Fix YAML.load_file failing on aliases [#234]

Added
- Make backward compatible changes to work with repmgr13 - version 5.2.1 [#192]
- Support Ruby 3.0 [#206]
- Support Ruby 3.1 [#227]
- Allow rails 7 gems in gemspec [#226]

Changed
- Update to Highline 2.1.0 [#201]
- Clean up test output (highline and stdout messages) [#210]

Removed
- Drop Ruby 2.7 [#223]
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