Re: [Tails-dev] [review'n'merge:1.2] test/7804-test-unit-ass…

Delete this message

Reply to this message
Author: anonym
Date:  
To: The Tails public development discussion list
Subject: Re: [Tails-dev] [review'n'merge:1.2] test/7804-test-unit-assertions [Was: Reasons to use our own assert() instead of Test::Unit::Assertions' one?]
18/09/14 18:20, intrigeri wrote:
> Hi,
>
> intrigeri wrote (08 Aug 2014 13:28:32 GMT) :
>> Is a reason why we use our own assert() function, instead of using
>> a library such as Test::Unit::Assertions?
>
>> If there's actually no good reason to do so, then I'm happy to pick an
>> adequate library and convert our test suite to it.
>
> Ticket: #7804 (should I really write this explicitly when the branch
> name already provides the info?)


In those cases, let's not.

> anonym agreed (privately, IIRC), and here's a branch that implements
> it. The initial patch was prepared by anonym, and then I added
> a commit on top to convert many assert() to assert_equal() and clean
> the code up accordingly. I've successfully run the test suite with
> these changes applied.
>
> Candidate for 1.2, assigned to anonym as I don't see who else is
> around and comfortable enough with the test suite code to review this,
> *but* I'd be delighted if someone else (Kill Your TV? bertagaz?)
> seized this opportunity to have a look and get more familiar with
> it :)


The function assert_equal is defined as (according to [1]):

    assert_equal( expected, actual, failure_message = nil )


but in every case in commit 5fe1539 it's done the other way around [2],
which would result in confusing output on assertion error unless a
failure_message is given. I've fixed this in commit XXX, and I was
overly pedantic and did it even when a failure_message is given, for
consistency.

I've merged this after carefully looking at my diff, and completing a
test run.

Cheers!

[1]
http://ruby-doc.org/stdlib-1.9.3/libdoc/test/unit/rdoc/Test/Unit/Assertions.html#method-i-assert_equal
[2] Which has the historical explanation that it made sense for `assert`
since it then can be read as "assert that X is blah". That said, I find
the argument order in `assert_equal` to be a bit confusing, but oh well.