The Katello pull request test runner is back and running as of 1pm EST
2/15/2014 and has begun to process all outstanding PRs. This breakage
resulted from a downstream break in our unit tests due to a recent commit
to Foreman core. For those interested, the details are as follows.
What broke things?
The Foreman pull request that broke the Katello tests was
https://github.com/theforeman/foreman/pull/1221 which enabled proper
permission checking on organization creation.
What was affected?
We had breakages across many tests that needed to create or save an
organization. A number of cases where this occurs:
Direct organization creation (e.g. Organization.create)
Creation of an organization through a Factory (e.g.
Saving an organization
This turned out to be an uncaught error on our side. For our use of the
Foreman organization fixture, we add a label the fixture and re-save the
organization. However, we were previously using the
save!. We made the false assumption that
:organization1was a valid (valid in the Rails model validation sense)
organization and thus our use of
savehid the fact that we weren't truly
saving the organization object. Thus, while the object itself would have a
label, any entity with a relationship to the organization that resulted in
fetching the organization would be incorrect.
How was this fixed?
Created an explicitly bare organization fixture in Foreman named
:empty_organizationand converted Katello to rely on this object
Anywhere an organization was created or saved, User.current was
explicitly set to the admin user and unset after create/save if a
previously set User.current existed.
Why didn't we use "as_admin"?
Currently, the Katello code does some thread session logic similar to
You will notice the check of
o.is_a?(self) which is used to ensure that
the object passed into this function is either a User object or an object
that inherits from User. In the Katello case, this is to allow system
registration via the CpConsumerUser class. The as_admin function makes use
of an un-instantiated fixture object that does not pass the test imposed by
the previously linked line of code. Thus, for now, we have resorted to
loading the fixture object from the database and setting User.current
explicitly. This does not impose significant test time increases.