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.

Sign up for GitHub

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

Jump to bottom

Fixes #30339 - isolate LookupKey tests #7808

Merged
merged 1 commit into from Jul 30, 2020
Merged

Fixes #30339 - isolate LookupKey tests #7808

merged 1 commit into from Jul 30, 2020

Conversation

ezr-ondrej
Copy link
Member

Gets rid of the PuppetclassLookupKey dependency for testing a LookupKey.
Removes some tests as they are irrelevant to LookupKey itself.

houndci-bot
houndci-bot reviewed Jul 8, 2020
View reviewed changes
test/unit/host_info_providers/puppet_info_test.rb Outdated Show resolved Hide resolved
test/unit/host_info_providers/puppet_info_test.rb Outdated Show resolved Hide resolved
test/unit/host_info_providers/puppet_info_test.rb Outdated Show resolved Hide resolved
test/models/lookup_key_test.rb Outdated Show resolved Hide resolved
@theforeman-bot
Copy link
Member

Issues: #30339

@ezr-ondrej ezr-ondrej force-pushed the isolate_lookup_key_test branch 4 times, most recently from 503eaba to 5548182 Compare July 8, 2020 12:49
@ezr-ondrej
Copy link
Member Author

[test foreman]

@ezr-ondrej ezr-ondrej mentioned this pull request Jul 20, 2020
Merged
kgaikwad
kgaikwad suggested changes Jul 29, 2020
View reviewed changes
Copy link
Member

@kgaikwad kgaikwad left a comment

Choose a reason for hiding this comment

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

@ezr-ondrej, added two small inline comments.

test/models/lookup_key_test.rb Outdated Show resolved Hide resolved
test/models/lookup_keys/puppetclass_lookup_key_test.rb Outdated Show resolved Hide resolved
@ezr-ondrej
Copy link
Member Author

Right, thanks @kgaikwad ! I've accepted your suggestions :) 👍

kgaikwad
kgaikwad approved these changes Jul 29, 2020
View reviewed changes
Copy link
Member

@kgaikwad kgaikwad left a comment

Choose a reason for hiding this comment

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

Codewise looks good to me 👍
I guess failing katello test is irrelevant to pull-request changes.

@shiramax, do you have any additional comments?

shiramax
shiramax approved these changes Jul 30, 2020
View reviewed changes
Copy link
Contributor

@shiramax shiramax left a comment

Choose a reason for hiding this comment

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

@ezr-ondrej looks good!

@shiramax
Copy link
Contributor

thanks @ezr-ondrej and @kgaikwad for the review :)

@shiramax shiramax merged commit c4c097f into theforeman:develop Jul 30, 2020
@ezr-ondrej ezr-ondrej deleted the isolate_lookup_key_test branch July 30, 2020 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@shiramax shiramax shiramax approved these changes

@kgaikwad kgaikwad kgaikwad approved these changes

Assignees

@shiramax shiramax

@kgaikwad kgaikwad

Labels
Needs testing
Projects
None yet
Milestone
No milestone
5 participants
@ezr-ondrej @theforeman-bot @shiramax @kgaikwad @houndci-bot