Hi,
I'm interested in doing a bit of Padre hacking. To get familiar with the
codebase, I picked a bug that I thought looked simple, and tried to fix it.
Turned out it was (patch attached).
This fixes the bug reported in Trac #896, relating to find history. There
was a small logic error causing the problem, which was actually more general
than reported. Search terms weren't getting added to the history in a lot of
cases.
I've also included a unit test, which makes use of Test::MockObject (I've
added this as a requirement in Makefile.PL). Hope the extra dep is OK... I
think T::MO will be pretty useful in a number of other cases, as a lot of
the constructors, base classes, etc. are quite heavy duty, and it can make
it difficult to test some simple logic without it.
Anyway, could I please also get a Trac account setup so I can attach patches
/ comments directly to the defects in future.
Thanks,
Sam.
Index: Makefile.PL
===================================================================
--- Makefile.PL (revision 11318)
+++ Makefile.PL (working copy)
@@ -165,6 +165,7 @@
requires 'Module::CoreList' => 0;
test_requires 'Capture::Tiny' => '0.06';
test_requires 'Test::More' => '0.88';
+test_requires 'Test::MockObject' => '1.09';
test_requires 'Test::Script' => '1.07';
test_requires 'Test::Exception' => '0.27';
test_requires 'Test::NoWarnings' => '0.084';
Index: t/74-history-combobox.t
===================================================================
--- t/74-history-combobox.t (revision 0)
+++ t/74-history-combobox.t (revision 0)
@@ -0,0 +1,65 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+use Test::More;
+
+BEGIN {
+ unless ( $ENV{DISPLAY} or $^O eq 'MSWin32' ) {
+ plan skip_all => 'Needs DISPLAY';
+ exit 0;
+ }
+ plan tests => 7;
+}
+
+use Test::NoWarnings;
+use t::lib::Padre;
+use Test::MockObject;
+
+use Padre::Wx;
+
+
+# Startup
+my $mock_history_combobox = Test::MockObject->new();
+$mock_history_combobox->set_isa('Wx::ComboBox');
+
+$mock_history_combobox->fake_module( 'Wx::ComboBox',
+ GetValue=> sub { return 'foo' }
+);
+
+use_ok 'Padre::Wx::History::ComboBox';
+
+SCOPE: {
+ # Check item added to history when not already found
+
+ # GIVEN
+ $mock_history_combobox->set_always('FindString', Wx::wxNOT_FOUND);
+ $mock_history_combobox->{type} = 'test1';
+
+ # WHEN
+ my $value = Padre::Wx::History::ComboBox::GetValue($mock_history_combobox);
+
+ # THEN
+ is($value, 'foo', "GetValue returned correct value");
+ my @history = Padre::DB::History->recent('test1');
+ is(scalar @history, 1, "One item in history list");
+ is($history[0], 'foo', "Correct value in history");
+}
+
+SCOPE: {
+ # Check item not added to history when already exists
+
+ # GIVEN
+ $mock_history_combobox->set_always('FindString', 0);
+ $mock_history_combobox->{type} = 'test2';
+
+ # WHEN
+ my $value = Padre::Wx::History::ComboBox::GetValue($mock_history_combobox);
+
+ # THEN
+ is($value, 'foo', "GetValue returned correct value");
+ my @history = Padre::DB::History->recent('test2');
+ is(scalar @history, 0, "Item not recorded in history");
+}
+
+1;
Index: lib/Padre/Wx/History/ComboBox.pm
===================================================================
--- lib/Padre/Wx/History/ComboBox.pm (revision 11318)
+++ lib/Padre/Wx/History/ComboBox.pm (working copy)
@@ -43,7 +43,7 @@
# If this is a value is not in our recent list, save it.
if ( defined $value and length $value ) {
- unless ( $self->FindString($value) == Wx::wxNOT_FOUND ) {
+ if ( $self->FindString($value) == Wx::wxNOT_FOUND ) {
Padre::DB::History->create(
type => $self->{type},
name => $value,
_______________________________________________
Padre-dev mailing list
Padre-dev@perlide.org
http://mail.perlide.org/mailman/listinfo/padre-dev
