fixes #8857 - refactor Debian boot_files_uri into parent class to match CoreOS #2053
Conversation
|
There were the following issues with the commit message:
Guidelines are available on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
johscheuer
force-pushed
the
fix-debian-boot_files_uri
branch
from
96b600c
to
a65b971
Compare
johscheuer
changed the title
|
[test] |
johscheuer
force-pushed
the
fix-debian-boot_files_uri
branch
from
a65b971
to
9adce26
Compare
|
[test] |
| @@ -0,0 +1,45 @@ | |||
| require 'test_helper' | |||
|
|
|||
| class DebianTest < ActiveSupport::TestCase | |||
There was a problem hiding this comment.
The name of this file has a typo, "debin" instead of "debian". When you correct it, please put the test in test/unit/operatingsystems/debian_test.rb, just to keep a consistent hierarchy with the one in app/models
|
@johscheuer Nice job! There are only a few issues with the way tests are structured in my opinion, it works well although I have not thoroughly tested it yet. See the comments above and I'll test it a bit more, but it looks like 👍 from my side. 😄 |
|
@elobato Thanks 👍 :) I will have a look and fix these things. |
johscheuer
force-pushed
the
fix-debian-boot_files_uri
branch
from
9adce26
to
774f1cb
Compare
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
johscheuer
force-pushed
the
fix-debian-boot_files_uri
branch
from
774f1cb
to
f7d0252
Compare
|
There were the following issues with the commit message:
Guidelines are available on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
johscheuer
force-pushed
the
fix-debian-boot_files_uri
branch
from
3449e8b
to
513cb5e
Compare
|
[test] |
johscheuer
force-pushed
the
fix-debian-boot_files_uri
branch
from
513cb5e
to
3d22c8e
Compare
| assert_equal 1, operatingsystems.count | ||
| assert_equal operatingsystems(:suse), operatingsystems.first | ||
| assert_equal operatingsystems(:centos5_3), operatingsystems.first | ||
| end |
There was a problem hiding this comment.
I changed the os from Suse to centos, so we can safely remove the fixture for Suse
|
[test] |
|
There were the following issues with the commit message:
Guidelines are available on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
…ass to match CoreOS Removed ubuntu_utopic from media.yml which caused the postgresql tests to fail Refactored unit tests Refactored os unit tests into one test file with param. tests Refactored usage of fixtures into factories Remove Checkstyle warnings and changed operatingsystem_test to use centos instead of suse fixed Lint/BlockAlignment
johscheuer
force-pushed
the
fix-debian-boot_files_uri
branch
from
6f17b0d
to
93e6a06
Compare
|
[test] |
|
Looks pretty good, I'll comment a few nitpicks I'll fix on merge just to leave some footprint. Just a sec... |
| @@ -252,7 +252,7 @@ def downcase_release_name | |||
| end | |||
|
|
|||
| def boot_files_uri(medium, architecture, host = nil) | |||
| raise (_("invalid medium for %s") % to_s) unless media.include?(medium) | |||
| raise ::Foreman::Exception.new(N_("invalid medium for %s"), to_s) unless media.include?(medium) | |||
There was a problem hiding this comment.
Change to_s to self. A rubocop cop we should use forces this, it's much more readable.
|
Merged as 03363cd, fixes in e760512, thanks @johscheuer! |
Refactor Debian boot_files_uri into parent class to match CoreOS. See Issue http://projects.theforeman.org/issues/8857