The Chainsaw Infanticide Logger Manuever

ruby-talk
21

Seems this thread just come over the fixed gateway? Seems to have
suddenly appeared with 34 post-fated messages in it. I've been missing
all the action! :slight_smile:

Anyway, If I may add 2c. A good way to cleanly etend classes is via
AOP. Now granted you can still run in the same problems but there's
full SOC so they are much easier to track down. In fact using AOP would
be preferable to eve extending classes expect in those very basic
pretty much guarunteed safe cases --that is, if Ruby had tight AOP
support which it doesn't.

Matz has suggested the :pre, :post, :wrap hooks, and those will
ceratinly help albeit these are not full SOC since they still represent
a reopinging of a class.

T.

22

#: Jeff Wood changed the world a bit at a time by saying on 8/24/2005 7:06 PM :#

The truely iterative & testing-complete way to build software is to
design and implement your tests before you write your code. When all
of your tests pass, you're done...

... If you built your system like that, you should have tests for each
and every action on your classes ... and the first time somebody elses
modifications has an affect on your functionality and expected output,
you should know about it.

j.

Why running away from the idea of the original mail? Having the tests may solve the problem. Or they may not: what if your tests are behaving correctly just because of the wrongly handled code outside of your control? Should you start to develop tests for that lib too? No.

When you expect something from a lib you take it as documented. If that lib does something undercover it should document it. It's simple. The tests have their power, but they are not the silver bullet.

respectfully,
:alex |.::the_mindstorm::.|

···

On 8/24/05, Bill Kelly <billk@cts.com> wrote:

From: "Jeff Wood" <jeff.darklight@gmail.com>
>
> You shouldn't be afraid of having power. That's why you have tests.
> You do have tests, right? ... right??? ... RIGHT !!?!?!?!??!

Dudes, I do have tests. How in blazes would you write a test
to catch the problem I described? (Please show your work. :slight_smile:
And why would you ever think to do so?

It's not being afraid of having power. In my own applications,
I'll exploit any and every cool capability of Ruby I feel like.

But when I personally write a module I view as a library I'd
like others to find useful, I take a decidedly more conservative
approach - deliberately.

Do you not consider libraries and applications different, where
Ruby's beloved Openness is concerned?

Regards,

Bill

23

Jeff,

The truely iterative & testing-complete way to build software is to
design and implement your tests before you write your code. When all
of your tests pass, you're done...

... If you built your system like that, you should have tests for each
and every action on your classes ... and the first time somebody elses
modifications has an affect on your functionality and expected output,
you should know about it.

I appreciate the well-meaning nature of your replies, but I've
been programming for 24 years and doing TDD semi-consistently
for the last 5.

Again, I'd ask to see what sort of test you'd propose that would
catch the problem I described. (My post, the one you quoted, was
about CGI, not the Logger.)

To answer "how/why would you test this"... you should have tests in
place for your logging functionality. The second something doesn't
come out right, you know that something is marring the system.
Debugging starts by walking the source of the packages you are using
and watching for references to the modified functionality... It's not
that hard to track down.

Thanks for the tip. =D

I also would ask again, Do you not consider libraries and applications
different, where Ruby's beloved Openness is concerned?

Regards,

Bill

···

From: "Jeff Wood" <jeff.darklight@gmail.com>

24

Jeff Wood wrote:

So,

To answer "how/why would you test this"... you should have tests in
place for your logging functionality. The second something doesn't
come out right, you know that something is marring the system. Debugging starts by walking the source of the packages you are using
and watching for references to the modified functionality... It's not
that hard to track down.

As Alexander mentioned, the point of the email was not that he couldn't detect it, it's that there's nothing he can do about it short of extending the Logger class *again* to make it do what he wants.

Unit tests detect the problem, they don't solve it. Since the OP controls neither the logger package nor the glue package, his choices are limited.

Plus, I'll second Alexander's assertion that you should never have to include unit tests to verify that classes work as advertised. That's the package author's job, and a programmer's job not to screw with their behavior, especially not classes in core and/or stdlib. The alternative is a horribly slippery slope.

Regards,

Dan

25

I'm not saying you need to test Logger ... I'm simply stating that you
should be asserting ALL of the behaviors you design into your
application.

What if logger isn't even being called ??? or any of a number of
situations like that... Simply verifying that the application behaves
in all ways as designed/required isn't "testing Logger".

j.

···

On 8/24/05, David Brady <ruby_talk@shinybit.com> wrote:

Eric Hodel wrote:

> I don't understand why I'm supposed to test a standard library I'm
> using. Usually I read the documentation and expect it to work that
> way, but maybe I'm crazy.

Me either. I thought Logger was your own code. I agree that you
shouldn't have to unit test a library that you trust.

If it's just Logger that's crap, then you can shrug and decide not to
trust the library and respond accordingly. But you're right, that Ruby
allows other programmers to open a library you trust and bash it into
untrustworthiness. That's a real problem that should be considered.

Is the following legal unit test code?

def test_no_messing_with_trusted_libraries
    require 'logger'
    Logger.freeze
    assert_nothing_raised( require 'suspect_module', "Hey! Somebody's
mucking with the logger module!" )
end

You're right, you shouldn't HAVE to run that test.

-dB

--
David Brady
ruby_talk@shinybit.com
I'm feeling really surreal today... OR AM I?

--
"So long, and thanks for all the fish"

Jeff Wood

26

[NOTE: This is an offshoot of Zed Shaw's rant on Chainsaw Infanticide.]

Austin Ziegler said:

You shouldn't be afraid of having power. That's why you have tests.
You do have tests, right? ... right??? ... RIGHT !!?!?!?!??!

I think the real problem is when this is done in released code. If
you're going to extend code, extend it cleanly -- IMO.

Well said.

It is probably worth having a public discussion on the meaning of
"extend cleanly".

Aye, so I'll start. If you're going to extend core or standard library
classes, you should:

1. Do so only at the user's request. Diff::LCS *can* extend String and
   Array but does not do so by default. If you are going to extend by
   default, then you must document it in a very loud tone of voice, as
   it were.

2. Don't depend on extensions to the core or standard library classes if
   you're working on a *library* of code for others to use. Subclass,
   extend (with a module), or delegate if you absolutely must. The
   predecessor to Diff::LCS (Algorithm::Diff) added #map_with_index or
   something similar to Array and depended on it. I don't think that
   Diff::LCS does that.

   Applications and application frameworks may have exemptions. This
   sort of allows for 1.day.ago notation as in Rails.

3. If you absolutely must extend the core and depend on it in a library,
   try to use names that don't interfere with others.
   Transaction::Simple follows #1, but it also follows this.

-austin

···

On 8/24/05, Jim Weirich <jim@weirichhouse.org> wrote:

On 8/24/05, Jeff Wood <jeff.darklight@gmail.com> wrote:

--
Austin Ziegler * halostatue@gmail.com
               * Alternate: austin@halostatue.ca

27

But therefore the converse is also true - it allows you to open untrustworthy code and bash it into trustworthiness :slight_smile:

Julian.

···

On 25/08/2005, at 4:54 AM, David Brady wrote:

I don't understand why I'm supposed to test a standard library I'm using. Usually I read the documentation and expect it to work that way, but maybe I'm crazy.

Me either. I thought Logger was your own code. I agree that you shouldn't have to unit test a library that you trust.

If it's just Logger that's crap, then you can shrug and decide not to trust the library and respond accordingly. But you're right, that Ruby allows other programmers to open a library you trust and bash it into untrustworthiness. That's a real problem that should be considered.

28

Ok, my set of tests for my application would include asserting that if
a given piece of functionality is going to cause a log message to
occur, that I validate the residual log files do contain the log
entries I expected as I expected them to be formatted. If they don't
then the application/library doesn't meet it's spec and/or
requirements. It's a bug and it needs to be resolved.

So, you would then have a front-row seat if something was affecting
the logger output.

And no, I don't believe there is any difference between building a
library and building an application ... it's all code that needs to be
tested and made sure it functions completely as designed.

I hope that answers you sufficiently.

j.

···

On 8/24/05, Bill Kelly <billk@cts.com> wrote:

Jeff,

From: "Jeff Wood" <jeff.darklight@gmail.com>

> The truely iterative & testing-complete way to build software is to
> design and implement your tests before you write your code. When all
> of your tests pass, you're done...
>
> ... If you built your system like that, you should have tests for each
> and every action on your classes ... and the first time somebody elses
> modifications has an affect on your functionality and expected output,
> you should know about it.

I appreciate the well-meaning nature of your replies, but I've
been programming for 24 years and doing TDD semi-consistently
for the last 5.

Again, I'd ask to see what sort of test you'd propose that would
catch the problem I described. (My post, the one you quoted, was
about CGI, not the Logger.)

> To answer "how/why would you test this"... you should have tests in
> place for your logging functionality. The second something doesn't
> come out right, you know that something is marring the system.
> Debugging starts by walking the source of the packages you are using
> and watching for references to the modified functionality... It's not
> that hard to track down.

Thanks for the tip. =D

I also would ask again, Do you not consider libraries and applications
different, where Ruby's beloved Openness is concerned?

Regards,

Bill

--
"So long, and thanks for all the fish"

Jeff Wood

29

Bill Kelly wrote:

Again, I'd ask to see what sort of test you'd propose that would
catch the problem I described. (My post, the one you quoted, was
about CGI, not the Logger.)

I find that the hardest tests to write are the ones that I overlooked because I am unaware of my blind spots.

Until I read your post, I thought like you did--that to_s flattened an object into a chunk of text. But now I see that it doesn't. It doesn't even return a String object! It returns an object that behaves--dare I say quacks?--like a String.

Another assumption you may be making--and once I state it, the test for it should become obvious--is that YAML should output pleasingly human readable text, even if it loses the ability to correctly restore objects.

I know, I know. There exists in business the concept of "approprately incorrect". You are using YAML to produce "appropriately incorrect" files.

The behavior you want is "take a Stringlike object, and emit pleasingly simplified text". That's easily testable. Take your CGI object, have YAML dump it, then open the YAML file manually and see that the text is correct. Or build a CGI object, then build an equivalent object using Strings, output them to different files and then assert that there is no difference between the files. The second method may seem more robust, because it lets you not care how YAML stores Strings... but that's the rabbit that led us down this hole in the first place. Use the first method, because you don't care if YAML stores CGI the same as a String, you care that the output is pleasing to read. If YAML changes the way it stores String, you'll want this test to break. Your app should shriek until you have verified that the new format is pleasant.

Once you have a regression test in place, you will be able to remove the hack to String#to_s and know when your application is fixed. A good alternative to your hack, perhaps, is to replace YAML with a dumper of your own devising that transforms Stringlike-->String on save/reload. Perhaps even a child class of YAML, whose only override is something like:

obj = String.new(obj) if obj.respond_to? :to_str && obj.class != String

I haven't learned how to see my blind spots yet. I could not have prevented this bug from happening. But having happened, I can immediately write a test that verifies that it is fixed, and that future revisions to CGI, YAML, and even String will be regression tested against this bug.

Cheers,

-dB

···

--
David Brady
ruby_talk@shinybit.com
I'm feeling really surreal today... OR AM I?

30

Austin Ziegler wrote:

Aye, so I'll start. If you're going to extend core or standard library
classes, you should:

1. Do so only at the user's request.

I disagree. requiring a library should extend stuff in the standard library if that is proper behavior for the library. E.g. require 'foo' might well inject a parsing ctor into String as String#to_foo, if it made sense to do so. I don't see any difference between a library and a framework extending Fixnum with #day as per Rails. I agree strongly with documenting it in a very loud tone of voice, however.

1.1 Do not change existing behavior of external classes or modules.
1.1.1 If you reopen an external class and redefine an existing method, should Ruby issue a warning? Is there a safe_level that will prevent a library from touching external classes?
1.1.2 Before adding a new method to a class or method, consider testing with respond_to? to see if it really is a new method, and raising a warning if it isn't.

4. In a library, never change existing behavior of code outside the library. If I have a set of unit tests for a class, and I require your library, all of the unit tests for my class must still pass.

It would be really useful if there were an idiomatic way of testing the Core, StdLib and any modules the library required. Then a sanity test could be run against the library verifying that all is as it should be. This wouldn't be a short unit test, but something akin to the exhaustive setup test you can run after building Ruby. It would be useful before *publishing* a library to validate its sanity. Perhaps if you propose to publish a module called foo, you may put test_foo.rb in the same directory, or in ../test/test_foo.rb, and for any modules you require that also provide a test_*.rb module, the sanity checker would run them as well. Finally, modules in the RAA or other major repositories could have the sanity checker's output placed alongside, letting the user know to what extent it has been tested.

-dB

···

--
David Brady
ruby_talk@shinybit.com
I'm feeling really surreal today... OR AM I?

31

#: Austin Ziegler changed the world a bit at a time by saying on 8/24/2005 10:31 PM :#

[NOTE: This is an offshoot of Zed Shaw's rant on Chainsaw Infanticide.]

It is probably worth having a public discussion on the meaning of
"extend cleanly".

Aye, so I'll start. If you're going to extend core or standard library
classes, you should:

1. Do so only at the user's request. Diff::LCS *can* extend String and
   Array but does not do so by default. If you are going to extend by
   default, then you must document it in a very loud tone of voice, as
   it were.

2. Don't depend on extensions to the core or standard library classes if
   you're working on a *library* of code for others to use. Subclass,
   extend (with a module), or delegate if you absolutely must. The
   predecessor to Diff::LCS (Algorithm::Diff) added #map_with_index or
   something similar to Array and depended on it. I don't think that
   Diff::LCS does that.

   Applications and application frameworks may have exemptions. This
   sort of allows for 1.day.ago notation as in Rails.

3. If you absolutely must extend the core and depend on it in a library,
   try to use names that don't interfere with others.
   Transaction::Simple follows #1, but it also follows this.

-austin

I will add my outsider 2c opinions:

1/ extend only if you don't have any other means to do it (meaning you have tried: subclass, extend with module or delegate)

2/ if extending than use non-conflicting names and document them

3/ if you reopen an external class and redefine existing methods: document this and offer an alias

These apply imo to all core, standard and frameworks. Applications would be nice to behave the same but not mandatory. However keep in mind that one row of documentation may change a whole day for somebody else. Respect the others!

:alex |.::the_mindstorm::.|

32

Hi David,

Thanks for your thoughtful reply.

Bill Kelly wrote:

>Again, I'd ask to see what sort of test you'd propose that would
>catch the problem I described. (My post, the one you quoted, was
>about CGI, not the Logger.)
>
>
I find that the hardest tests to write are the ones that I overlooked
because I am unaware of my blind spots.

Yes - agreed.

Until I read your post, I thought like you did--that to_s flattened an
object into a chunk of text. But now I see that it doesn't. It doesn't
even return a String object! It returns an object that behaves--dare I
say quacks?--like a String.

Another assumption you may be making--and once I state it, the test for
it should become obvious--is that YAML should output pleasingly human
readable text, even if it loses the ability to correctly restore objects.

Hmm... Actually my assumption (or hope) would be that YAML always
be able to write out objects that it can correctly restore. My
reason for wanting my application to be serializing "real" simple
strings isn't really tied, I think, to any assumptions about how
YAML itself should behave.

But yes, I want my *application* to, using YAML, output simple
strings that are not only pleasingly human readable, but also
compact*. I have no problem with YAML doing exactly what I've
asked it to. I just didn't realize I was feeding it complex
singleton String instances with "extra" instance variables.

(*) The compactness is desirable for performance reasons:
I'm already splitting my databases via hashing into 4093 sub-
files, so each component file of the database can be loaded/saved
quickly enough. YAML Ain't Mercurial Lightning... ? :wink:

I know, I know. There exists in business the concept of "approprately
incorrect". You are using YAML to produce "appropriately incorrect" files.

I'm not sure I agree. I'm using YAML to accurately serialize my
data. It's my fault for feeding it unexpectedly complex data.

The behavior you want is "take a Stringlike object, and emit pleasingly
simplified text". That's easily testable. Take your CGI object, have
YAML dump it, then open the YAML file manually and see that the text is
correct. Or build a CGI object, then build an equivalent object using
Strings, output them to different files and then assert that there is no
difference between the files. The second method may seem more robust,
because it lets you not care how YAML stores Strings... but that's the
rabbit that led us down this hole in the first place. Use the first
method, because you don't care if YAML stores CGI the same as a String,
you care that the output is pleasing to read. If YAML changes the way
it stores String, you'll want this test to break. Your app should
shriek until you have verified that the new format is pleasant.

One test I had in mind was,

assert( cgi['something'].to_s.to_yaml ==
        String.new(cgi['something'].to_s).to_yaml )

...but that test does rely on my modified String#to_s behavior.

One complication is that, for the time being, my application needs
to run on systems with the 1.8.1 CGI behavior (which returns a
parameter value as a non-String-object that quacks like a String),
and the 1.8.2 CGI behavior (which returns a paramater value as a
genuine String instance whose singleton class happens to have been
extend'ed with a module that includes some bonus instance variables.)

So far, my String#to_s hack still seems like a reasonable enough
way to enforce the behavior I need in my current application. (But
again, I'd NOT take this hack approach if I were writing a library.)

So, given my String#to_s hack, I think the above assert() should
cover the specific problem I was having.

A more direct approach might have been for me to open the CGI class
and replace CGI# so that it returned simple String instances, not
complex extend'ed ones. (Again, something I'd consider for my app,
but not for a library.)

I'm not sure if more general tests involving YAML and Strings would
help, since I don't *really* want to change YAML's behavior - I just
want to not accidentally feed it complex objects.

Regards,

Bill

···

From: "David Brady" <ruby_talk@shinybit.com>

33

Austin Ziegler wrote:

Aye, so I'll start. If you're going to extend core or standard
library classes, you should:
1. Do so only at the user's request.

I disagree. requiring a library should extend stuff in the standard
library if that is proper behavior for the library. E.g. require 'foo'
might well inject a parsing ctor into String as String#to_foo, if it
made sense to do so. I don't see any difference between a library and
a framework extending Fixnum with #day as per Rails. I agree strongly
with documenting it in a very loud tone of voice, however.

I'm arguing that it's generally inappropriate for a random library to
add #day to Fixnum or to require a library which does so. See, I *do*
see a big difference between a library and an application framework. It
is inappropriate for a random library (say, Diff::LCS) to add a new
method to String and Array (#diff, as well as others). However, if one
is programming a Rails or Nitro application, then one is dealing with
Rails or Nitro and are, effectively, extending a *static* environment
that you control. Adding #day to Fixnum when you're a library, on the
other hand, is modifying an environment that you *don't* control (and
shouldn't).

Sometimes, as in #to_foo, it may be appropriate, but that should
*probably* be done with:

  require 'foo'
  require 'foo/string'

Alternatively, just use "require 'foo/string'" and 'foo/string' requires
'foo'. This is, IIRC, what Diff::LCS does.

Like I said, I'm not as hard and fast on this, but as a general rule,
one should avoid modifying those unless the user of the *library*
requests it.

1.1 Do not change existing behavior of external classes or modules.

I would generally agree with this.

1.1.1 If you reopen an external class and redefine an existing method,
should Ruby issue a warning? Is there a safe_level that will prevent a
library from touching external classes?

Ruby does issue a warning, when run with -w. And AFAIK, there is no such
$SAFE level that won't also restrict certain behaviours severely.

1.1.2 Before adding a new method to a class or method, consider
testing with respond_to? to see if it really is a new method, and
raising a warning if it isn't.

Maybe.

4. In a library, never change existing behavior of code outside the
library. If I have a set of unit tests for a class, and I require your
library, all of the unit tests for my class must still pass.

Agreed.

It would be really useful if there were an idiomatic way of testing
the Core, StdLib and any modules the library required. Then a sanity
test could be run against the library verifying that all is as it
should be.

Ultimately, this should be the Rubicon.

-austin

···

On 8/24/05, David Brady <ruby_talk@shinybit.com> wrote:
--
Austin Ziegler * halostatue@gmail.com
               * Alternate: austin@halostatue.ca

34

... which would lead you on the 3 hour hunt that Zed was complaining about originally, to finally find the problem was someone else's library.

You'd know just about as instantly as Zed did that something was wrong. And you'd still have no idea what was causing the bug, or how to fix it.

Unit tests are great, but I fail to see how they would have undone ANY of the harm that the original library did, or how they would have made Zed's life any easier in this one case.

···

On Aug 24, 2005, at 11:55 AM, Jeff Wood wrote:

So, you would then have a front-row seat if something was affecting
the logger output.

35

Ok, my set of tests for my application would include asserting that if
a given piece of functionality is going to cause a log message to
occur, that I validate the residual log files do contain the log
entries I expected as I expected them to be formatted. If they don't
then the application/library doesn't meet it's spec and/or
requirements. It's a bug and it needs to be resolved.

So, you would then have a front-row seat if something was affecting
the logger output.

At some point--and that point approaches us very quickly on anything
larger than a trivial project--we have to trust that our libraries
work as documented and that others won't deliberately mess our
environment up. Extension of a class to have a to_whatever is one
thing, changing behavior that was there before is another.

So, I'm not sure that testing this kind of thing is even reasonable.
It might be nice to have, but part of the point of TDD is to keep you
on track and developing at a consistent pace. If I stop to check every
method I might use, I'm going to get sidetracked far too quickly in
a language like Ruby

I think the moral of the story is that Logger needs a meta channel
which records the state of the logger itself. I've used loggers that
have such a feature and it's surprisingly useful.

···

On Aug 24, 2005, at 10:55 AM, Jeff Wood wrote:

And no, I don't believe there is any difference between building a
library and building an application ... it's all code that needs to be
tested and made sure it functions completely as designed.

I hope that answers you sufficiently.

j.

On 8/24/05, Bill Kelly <billk@cts.com> wrote:

Jeff,

From: "Jeff Wood" <jeff.darklight@gmail.com>

The truely iterative & testing-complete way to build software is to
design and implement your tests before you write your code. When all
of your tests pass, you're done...

... If you built your system like that, you should have tests for each
and every action on your classes ... and the first time somebody elses
modifications has an affect on your functionality and expected output,
you should know about it.

I appreciate the well-meaning nature of your replies, but I've
been programming for 24 years and doing TDD semi-consistently
for the last 5.

Again, I'd ask to see what sort of test you'd propose that would
catch the problem I described. (My post, the one you quoted, was
about CGI, not the Logger.)

To answer "how/why would you test this"... you should have tests in
place for your logging functionality. The second something doesn't
come out right, you know that something is marring the system.
Debugging starts by walking the source of the packages you are using
and watching for references to the modified functionality... It's not
that hard to track down.

Thanks for the tip. =D

I also would ask again, Do you not consider libraries and applications
different, where Ruby's beloved Openness is concerned?

Regards,

Bill

--
"So long, and thanks for all the fish"

Jeff Wood

--
Dave Fayram <dfayram@gmail.com>
Software Developer

36

Austin Ziegler wrote:

Sometimes, as in #to_foo, it may be appropriate, but that should
*probably* be done with:

require 'foo'
require 'foo/string'

I *REALLY* like this idiom!

require 'mymodule/standardmodule' # allow mymodule to make injections to standardmodule.

Nifty!

-dB

···

--
David Brady
ruby_talk@shinybit.com
I'm feeling really surreal today... OR AM I?

37

Thanks. I didn't invent it, but it seems the *best* way to solve what
I'd said re: extending cleanly.

If Rails wanted to do this (as a library, not a framework), then it
might be require 'activerecord/date/fixnum' or something like that.
But that's just what *I* would do, and not what everyone would do.

-austin

···

On 8/24/05, David Brady <ruby_talk@shinybit.com> wrote:

Austin Ziegler wrote:
>Sometimes, as in #to_foo, it may be appropriate, but that should
>*probably* be done with:
>
> require 'foo'
> require 'foo/string'
>
>
I *REALLY* like this idiom!

require 'mymodule/standardmodule' # allow mymodule to make injections to
standardmodule.

Nifty!

--
Austin Ziegler * halostatue@gmail.com
               * Alternate: austin@halostatue.ca