[PATCH evemu 10/19] tests: Fix tests with fixed lists

[Peter Hutterer]

On Tue, Jan 07, 2014 at 10:15:58AM +0100, Daniel Martin wrote:
> On Tue, Jan 07, 2014 at 09:17:39AM +1000, Peter Hutterer wrote:
> > On Mon, Jan 06, 2014 at 06:38:10PM +0100, Daniel Martin wrote:
> > > Comparing a fixed list of values with a list of values which have been
> > > computed out of an _unordered_ dictionary is a bad idea. A simple fix
> > > would've been to use an OrderedDict.
> > > 
> > > Instead, replace the fixed lists of values with descriptive dictionaries
> > > and use those dictionaries to generate the expected values in tests for
> > > the comparisions.
> > > 
> > > v2: Use constants as keys in _expected_{abs,key}_ntrig_dell_xt2 and make
> > >     the dictionary comprehension Python 2.6 compatible.
> > > 
> > > Signed-off-by: Daniel Martin <consume.noise at gmail.com>
> > > ---
> > >  python/evemu/testing/testcase.py  | 51 +++++++++++++++++++++
> > >  python/evemu/tests/test_device.py | 96 ++++++++++++++-------------------------
> > >  2 files changed, 85 insertions(+), 62 deletions(-)
> > > 
> > > diff --git a/python/evemu/testing/testcase.py b/python/evemu/testing/testcase.py
> > > index f19504a..2603581 100644
> > > --- a/python/evemu/testing/testcase.py
> > > +++ b/python/evemu/testing/testcase.py
> > > @@ -61,3 +61,54 @@ class BaseTestCase(unittest.TestCase):
> > >      def get_events_file(self):
> > >          return os.path.join(self.data_dir, "ntrig-dell-xt2.event")
> > >  
> > > +    _expected_abs_ntrig_dell_xt2 = {
> > > +            # A: 00 0 9600 75 0 0
> > > +            const.absolute_axes["ABS_X"]:
> > > +                {"min": 0, "max": 9600, "fuzz": 75, "flat": 0, "res": 0},
> > > +            # A: 01 0 7200 78 0 0
> > > +            const.absolute_axes["ABS_Y"]:
> > > +                {"min": 0, "max": 7200, "fuzz": 78, "flat": 0, "res": 0},
> > > +            # A: 30 0 9600 200 0 0
> > > +            const.absolute_axes["ABS_MT_TOUCH_MAJOR"]:
> > > +                {"min": 0, "max": 9600, "fuzz": 200, "flat": 0, "res": 0},
> > > +            # A: 31 0 7200 150 0 0
> > > +            const.absolute_axes["ABS_MT_TOUCH_MINOR"]:
> > > +                {"min": 0, "max": 7200, "fuzz": 150, "flat": 0, "res": 0},
> > > +            # A: 34 0 1 0 0 0
> > > +            const.absolute_axes["ABS_MT_ORIENTATION"]:
> > > +                {"min": 0, "max": 1, "fuzz": 0, "flat": 0, "res": 0},
> > > +            # A: 35 0 9600 75 0 0
> > > +            const.absolute_axes["ABS_MT_POSITION_X"]:
> > > +                {"min": 0, "max": 9600, "fuzz": 75, "flat": 0, "res": 0},
> > > +            # A: 36 0 7200 78 0 0
> > > +            const.absolute_axes["ABS_MT_POSITION_Y"]:
> > > +                {"min": 0, "max": 7200, "fuzz": 78,  "flat": 0, "res": 0}
> > > +            }
> > > +    _expected_key_ntrig_dell_xt2 = {
> > > +            const.buttons["BTN_TOUCH"]: True
> > > +            }
> > > +
> > > +    def get_expected_abs(self, sub_key):
> > > +        expected_items = self._expected_abs_ntrig_dell_xt2.items()
> > > +        expected = dict.fromkeys(const.absolute_axes.values(), 0)
> > > +        expected.update((k, v[sub_key]) for (k, v) in expected_items)
> > > +
> > > +        return expected
> > > +
> > > +    def get_expected_ev_abs(self):
> > > +        expected_keys = self._expected_abs_ntrig_dell_xt2.keys()
> > > +        expected = dict.fromkeys(const.absolute_axes.values(), False)
> > > +        expected.update((k, True) for k in expected_keys)
> > > +
> > > +        return expected
> > > +
> > > +    def get_expected_ev_key(self):
> > 
> > sorry, should've seen that in the first version: I think the naming here
> > would be better as get_expected_absbits and get_expected_keybits,
> > .propbits.
> 
> What would be a proper renaming of the tests
>     test_has_event_ev_abs -> test_has_event_absbits
> or
>     test_has_event_ev_abs -> test_has_absbits?
> 
> > > +        expected_keys = self._expected_key_ntrig_dell_xt2.keys()
> > > +        expected = dict.fromkeys(const.buttons.values(), False)
> > > +        expected.update((k, True) for k in expected_keys)
> > > +
> > > +        return expected
> > > +
> > > +    def get_expected_prop(self):
> > > +        # no props for N-Trig-MultiTouch-Virtual-Device
> > > +        return dict.fromkeys(const.absolute_axes.values(), False)
> > 
> > this isn't a comment on your patch, more for the archives and future work:
> > I'd really prefer if for the future the BaseTestCase is abstracted in a way
> > that the n-trig-specific data is in an n-trig specifica class and object so
> > that we can add/remove devices and have device-specific data as such.
> > the code would roughly be something like this:
> > 
> >   class BaseTestClass:
> >       def setUp():
> >           self.sourcedevice = new TestDevice("ntrig-dell-xt2")
> > 
> >       def get_device_file(self):
> >           return self.sourcedevice.get_source_file()
> > 
> >       def get_expected_keybits(self):
> >           return self.sourcedevice.keybits()
> >
> > if anyone thinks of expanding the test suite, this would be a good next step
> > because it would allow adding more devices easily and just re-running the
> > tests for multiple devices.
> 
> I thought about that too. Would it be sufficient to specify the expected
> values for a device as I did in a dictionary?

yeah, pretty much just as you did but instead of having one device in a
base-class having the devices in separate classes each. or even nested
dicts? either way, some easy way to add more devices.

> The alternative would be to have another parser in the test suite. This
> would make it very easy to add more tests for devices, because only the
> property file to use would need to be specified. But, it requires more
> work if the property file format changes.
> 
> So, more work to do when adding a new device (transform a property file
> to a dictionary) or more work if the file format changes?

please let's not do another parser. we have evemu/test/ that we can (and
should) use for proper library testing. This part here are the python bits
only and I'm not even sure if anyone still uses them. Last I heard some CI
process inside Canonical somewhere used them, that's why they exist in the
first place.

Cheers,
   Peter