Test fix; inactive works a little differently now #33

merged 1 commit into from Aug 6, 2011


None yet
2 participants

gregheo commented Jul 26, 2011

inactive => 1 sets active => 0 and then deletes inactive; this updated test reflects that.

See: d59fc14

theory commented on 00cbed5 Jul 26, 2011

Do we need to add tests to properly cover how active and inactive are actually used?



gregheo replied Aug 6, 2011

Sorry, new mail filtering rules are a little too aggressive. I'm still here!

I think the existing tests are OK. The only change was the inactive key gets cleaned up but the test here and the one around line 144 should cover things.

The tests are not okay, alas. In rev-2.0, I get this failure:

not ok 1279 - inactive sets active 0
#   Failed test 'inactive sets active 0'
#   at t/Bric/Util/DBI/Test.pm line 435.
#   (in Bric::Util::DBI::Test->testclean_params)
#     Structures begin differing at:
#          $got->{inactive} = Does not exist
#     $expected->{inactive} = '1'

Am I missing something? I'm not clear on why this behavior has changed; can you explain? (And sorry if you have already, I either missed it or don't remember. Yes, I suck.)

Oh, I guess I just need to cherry-pick this commit. Sorry.

Still, what has changed here, and why?


gregheo replied Aug 17, 2011

inactive => 1 used to set active => 0 and leave the inactive key in the hash. My memory is a little fuzzy but I think the original bug was that active stories were still included in the search results since inactive is mapped to active a little deeper down in the bowels of the code.

So the fix was if inactive => 1 is present, set active => 0 and then delete the inactive key. This commit here updates the test to no longer expect inactive to be there.

I just noticed Mike Raynham reported and fixed this in his own fork and filed bug report #266 (that can probably be closed now).

Great, thanks for the explanation. I cherry-picked your patch and all tests pass now.

theory merged commit 2c497e0 into bricoleurs:master Aug 6, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment