refs #17201 - add rspec tests for migrations #425
Conversation
end | ||
|
||
after(:each) do | ||
`git reset HEAD --hard` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may surprise developers who run tests. Maybe assert git status --short config/
is empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, good point. I'll just fix it so the test migrations don't dump their answers back into the git repo, which is the only reason I have the reset.
0ba1957
to
347b076
Compare
@ekohl Updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Feel free to merge once the tests pass.
let(:migrator) { Kafo::Migrations.new(scenario[:migrations]).run(scenario[:config].dup, scenario[:answers].dup) } | ||
|
||
it 'does not change scenario config' do | ||
after, _ = migrator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rubocop doesn't like this
Thanks! |
Thanks! This also adds tests to all our migrations which I believe we didn't have at first. That's a cool addition. |
Indeed, maybe it's useful to foreman-installer? theforeman/foreman-installer#208 |
Refs #12556 - fix content host reference
No description provided.