What is the right way to monkey patch rails?

Sam Clearman wrote:

Right now the way i do it is i have at the end of my application.rb file

module ActionController::HttpAuthentication::Basic
  def authorization
    #code here...
  end
end

Is there a better way of doing it?

That looks like a standard Monkey Patch.

Got unit tests for it?

Phlip wrote:

Sam Clearman wrote:

Right now the way i do it is i have at the end of my application.rb file

module ActionController::HttpAuthentication::Basic
  def authorization
    #code here...
  end
end

Is there a better way of doing it?

That looks like a standard Monkey Patch.

Got unit tests for it?

--
   Phlip

hehe... nope.

Sam Clearman wrote:

Is there a better way of doing it?

That looks like a standard Monkey Patch.

Got unit tests for it?

hehe... nope.

Ordinarily, the correct way to do anything - especially an MP - is to unit test it.

However, in the case of Basic Authentication, you would need to mock the server... and the authorizer... and so on. The mocks would generally tell your test what they think it wants to hear, and you'd be back to square one.

So maybe the question how to monkey-patch something is isomorphic to the question how you reasonably test it.

Just make the monkey patch as small as possible, and then submit it to the maintainers... C-:

Hi --

Sam Clearman wrote:

Is there a better way of doing it?

That looks like a standard Monkey Patch.

Got unit tests for it?

hehe... nope.

Ordinarily, the correct way to do anything - especially an MP - is to unit test it.

However, in the case of Basic Authentication, you would need to mock the
server... and the authorizer... and so on. The mocks would generally tell your
test what they think it wants to hear, and you'd be back to square one.

So maybe the question how to monkey-patch something is isomorphic to the
question how you reasonably test it.

It's possible to write your model classes inside a view, and then test
them, but I still wouldn't recommend it :slight_smile: I'm all for testing, but I
don't think that discussing best practices in other terms has been
discredited. Also, there are tests for this class already, which could
probably be adapted (depending what the new version is doing, of
course).

David

David A. Black wrote:

It's possible to write your model classes inside a view, and then test
them, but I still wouldn't recommend it :slight_smile: I'm all for testing, but I
don't think that discussing best practices in other terms has been
discredited. Also, there are tests for this class already, which could
probably be adapted (depending what the new version is doing, of
course).

You are answering the question "how can I use tests to help bad
designs live longer?"

That's a valid question and answer, but it's non-responsive to my
point.

The goal, if you _must_ Monkey Patch, is to slightly increase the odds
of a clear diagnostic error message when the real library upgrades out
from under the patch.

Hi --

David A. Black wrote:

It's possible to write your model classes inside a view, and then test
them, but I still wouldn't recommend it :slight_smile: I'm all for testing, but I
don't think that discussing best practices in other terms has been
discredited. Also, there are tests for this class already, which could
probably be adapted (depending what the new version is doing, of
course).

You are answering the question "how can I use tests to help bad
designs live longer?"

No, neither asking nor answering that question :slight_smile: I just mean that I
don't think that "Can you write a 'reasonable' test suite for it?" is
the beginning and end of all examination of whether a given practice
is a good one. Among other things, test suites are constrained by test
tools, the current state of which I admire greatly but which are
themselves subject to change and modification. I think one has to take
all of this into account. That's all I mean.

That's a valid question and answer, but it's non-responsive to my
point.

The goal, if you _must_ Monkey Patch, is to slightly increase the odds
of a clear diagnostic error message when the real library upgrades out
from under the patch.

Sure, but that's true of all your code. I don't like to stigmatize
code that does something to the framework itself, which, like all
other code, can be good or bad, well-maintained or not, etc. The
adoption of the term "monkey patch" over the last couple of years is,
I think, unfortunate, since it does tend to stigmatize practices that
in many cases are perfectly reasonable and rational.

In short, I don't literally append "... but you have to do it well and
make sure it works" to everything I say about Ruby, Rails, or software
in general, but I think it's kind of understood to be implied :slight_smile:

David