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

Fixes #19741 - Undefined method split for nil on tests #6807

Merged
merged 1 commit into from
Jun 5, 2017

Conversation

dLobatog
Copy link
Member

@dLobatog dLobatog commented Jun 2, 2017

Due to a bug on minitest that is being currently fixed, our tests are
broken when they run as the "test_develop_pr_katello" job in Jenkins.

The fix is published as minitest/minitest#696
however there is no version of minitest with it yet. A possible
workaround is to prevent these assertions like this PR does.

Due to a bug on minitest that is being currently fixed, our tests are
broken when they run as the "test_develop_pr_katello" job in Jenkins.

The fix is published as minitest/minitest#696
however there is no version of minitest with it yet. A possible
workaround is to prevent these assertions like this PR does.
@dLobatog
Copy link
Member Author

dLobatog commented Jun 2, 2017

There is a job http://ci.theforeman.org/job/test_develop_pr_katello/2894/ running the affected Jenkins job with this PR, it should hopefully confirm that the workaround is valid.

@dLobatog
Copy link
Member Author

dLobatog commented Jun 2, 2017

Apparently the job needs a PR in Foreman to work too, not sure how to verify this.

Copy link
Member

@sean797 sean797 left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this @dLobatog ! 👍

EDIT: I reproduce this locally and this seem to fix it... I have no idea about how to get our Jenkins to verify

@dLobatog
Copy link
Member Author

dLobatog commented Jun 3, 2017

@sean797 How do you reproduce it locally?

@sean797
Copy link
Member

sean797 commented Jun 3, 2017

Sorry @dLobatog when I said locally I meant with another Jenkins.. At the moment I have access to lots of massively overspec'ed and underutilized Jenkins workers so I setup jobs from https://github.com/theforeman/foreman-infra while I was testing another PR. My laptop sounds like an airplane taking off if I run the whole tests suit locally ✈️

This also for example fixes: (which is nice)

DEPRECATED: Use assert_nil if expecting nil from /home/vagrant/katello/test/models/sync_plan_test.rb:35. This will fail in Minitest 6.

@lzap
Copy link
Contributor

lzap commented Jun 5, 2017

Finally, thanks!

@lzap
Copy link
Contributor

lzap commented Jun 5, 2017

Quick merge anyone? This is bugging me for weeks now! Thanks. @ehelms @jlsherrill @parthaa

@jlsherrill
Copy link
Member

ACK thanks @lzap !

@jlsherrill jlsherrill merged commit 885e3f2 into Katello:master Jun 5, 2017
@dLobatog
Copy link
Member Author

dLobatog commented Jun 5, 2017

\o/

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.

5 participants