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 broken ci.yaml #766

Merged
merged 1 commit into from
Nov 15, 2024
Merged

Fix broken ci.yaml #766

merged 1 commit into from
Nov 15, 2024

Conversation

Fryguy
Copy link
Member

@Fryguy Fryguy commented Nov 15, 2024

Introduced in #765

@kbrock or @jrafanie Please review.

#765 never actually ran the tests, so it "appeared" to be green even though it never actually ran.

Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

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

Off by one errors!

@jrafanie
Copy link
Member

WAT is that failure

1) ReencryptPasswordScramsha#up Ensures that the user password is stored as scram-sha-256
     Failure/Error: expect(ActiveRecord::Base.connection_db_config).to receive(:configuration_hash).exactly(10).times.and_call_original

       (#<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007f4cdb38ad18 @env_name="test", @name="primary", @configuration_hash={:adapter=>"postgresql", :encoding=>"utf8", :username=>"root", :***"smartvm", :pool=>3, :wait_timeout=>5, :min_messages=>"warning", :database=>"dummy_test"}>).configuration_hash(*(any args))
           expected: 10 times with any arguments
           received: 1 time with any arguments
     # ./spec/migrations/20241017013023_reencrypt_password_scramsha_spec.rb:12:in `block (3 levels) in <top (required)>'
     # ./spec/support/migration_helper.rb:77:in `clearing_caches'
     # ./spec/support/migration_helper.rb:13:in `block (2 levels) in migration_context'

  2) ReencryptPasswordScramsha#up Handles connections with no password
     Failure/Error: super

       (#<ActiveRecord::ConnectionAdapters::PostgreSQLAdapter:0x00007f4cda51acb0 @transaction_manager=#<ActiveRecord::ConnectionAdapters::TransactionManager:0x00007f4cda920a80 @stack=[#<ActiveRecord::ConnectionAdapters::RealTransaction:0x00007f4cd3e9c[47](https://github.com/ManageIQ/manageiq-schema/actions/runs/11858845607/job/33050405714?pr=766#step:6:48)0 @connection=#<ActiveRecord::ConnectionAdapters::PostgreSQLAdapter:0x00007f4cda51acb0 ...>, @state=#<ActiveRecord::ConnectionAdapters::TransactionState:0x00007f4cd3e9c420 @state=nil, @children=[#<ActiveRecord::ConnectionAdapters::TransactionState:0x00007f4cd395a930 @state=nil, @children=nil>]>, @records=nil, @isolation_level=nil, @materialized=true, @joinable=false, @run_commit_callbacks=true, @lazy_enrollment_records=nil, @dirty=true, @instrumenter=#<ActiveRecord::ConnectionAdapters::TransactionInstrumenter:0x00007f4cd3e9c290 @handle=#<ActiveSupport::Notifications::Fanout::Handle:0x00007f4cd3ea1718 @name="transaction.active_record", @id="4321fe7d6758014edd25", @payload={:connection=>#<ActiveRecord::ConnectionAdapters::PostgreSQLAdapter:0x00007f4cda[51](https://github.com/ManageIQ/manageiq-schema/actions/runs/11858845607/job/33050405714?pr=766#step:6:52)acb0 ...>}, @groups=[],

@Fryguy
Copy link
Member Author

Fryguy commented Nov 15, 2024

@bdunne might remember - I can't remember why we needed the 10 times

@Fryguy
Copy link
Member Author

Fryguy commented Nov 15, 2024

@jrafanie Does it make sense to merge this obvious syntax error as is (and thus actually run the tests), and then we fix master separately?

@jrafanie
Copy link
Member

@jrafanie Does it make sense to merge this obvious syntax error as is (and thus actually run the tests), and then we fix master separately?

yes

Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

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

merging with obvious syntax fix, we'll tackle the unrelated test failure separately

@jrafanie jrafanie merged commit 468dd18 into ManageIQ:master Nov 15, 2024
1 of 7 checks passed
@Fryguy Fryguy deleted the fix_broken_ci_yaml branch November 15, 2024 17:07
@Fryguy
Copy link
Member Author

Fryguy commented Nov 15, 2024

Backported to radjabov in commit 3b8b017.

commit 3b8b0172e470ff0ba29829fef6b0f954f1c86e70
Author: Joe Rafaniello <[email protected]>
Date:   Fri Nov 15 11:55:55 2024 -0500

    Merge pull request #766 from Fryguy/fix_broken_ci_yaml
    
    Fix broken ci.yaml
    
    (cherry picked from commit 468dd18f082447b25395686f11549a2c35a48dc8)

Fryguy pushed a commit that referenced this pull request Nov 15, 2024
Fix broken ci.yaml

(cherry picked from commit 468dd18)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants