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

Avoid crash if failed to resolve the IP from install.inf (bsc#1161217) #1028

Merged
merged 4 commits into from Jan 23, 2020

Conversation

ancorgs
Copy link
Contributor

@ancorgs ancorgs commented Jan 22, 2020

If install.inf contains an IP in the "hostname" field, YaST tries to resolve that to get a valid hostname. But there was a bug and if that resolution was not possible, YaST was crashing.

See https://bugzilla.suse.com/show_bug.cgi?id=1161217

There was a test to verify the correct behavior... but the test was as broken as the code.

In addition, I detected a dummy test that was testing the behavior of a fully mocked method. In the first two commits of this pull request, I tried to turn that into a reasonable test that, hopefully, fits the original intention of the test.

The test was testing a method that was mocked, changed for a more
reasonable test that probably honors the original intention.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 69.053% when pulling 930eaae on ancorgs:fix_1161217 into 2065a05 on yast:master.

Copy link
Contributor

@teclator teclator left a comment

Choose a reason for hiding this comment

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

LGTM

@ancorgs ancorgs merged commit 7cbbdac into yast:master Jan 23, 2020
@ancorgs ancorgs deleted the fix_1161217 branch January 23, 2020 08:52
@yast-bot
Copy link
Contributor

✔️ Internal Jenkins job #56 successfully finished
✔️ Created IBS submit request #209920

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.

None yet

4 participants