Refactor and add tests for plugins.py #642

SRomansky · 2014-11-02T00:16:07Z

Improvement to the tests currently being run on plugins.py

related #644

dideler · 2014-11-03T23:26:42Z

src/freeseer/tests/framework/test_plugins.py

+    for plugin in plugins:
+        plugin.plugin_object.load_config(plugin_manager)
+        if plugin.name == "Firewire Source":
+            # There is an issue with Firewire Source in testing


This would be better as a FIXME comment. Also refer to any related issues on GH if they exist.

sorry, what exactly is the issue?

Edit: can you explain the issue in the comment and also link to relevant bug numbers so that the next person who looks at this code has a fighting chance of understanding it?

I am not sure what the bug is. I thought https://github.com/Freeseer/freeseer/pull/589/files had discovered the bug but it appears it has not. I will look through git history to figure out when the comment was put it.

edit:
It appears that the firewire issue comment has existed since Freeseer was switch to git revision system.

The follow error is thrown when the firewire conditional is removed from the loop. This error occurs in the firewire src plugin based when constructing the pipe for GStreamer.

=================================== FAILURES =================================== _______________ test_plugin_bin[plugin_info2-expected_instance2] _______________ plugin_manager = <freeseer.framework.plugin.PluginManager object at 0x7f04682b9a68> plugin_info = ('VideoInput', 'get_videoinput_bin') expected_instance = <type 'gst.Bin'> @pytest.mark.parametrize('plugin_info, expected_instance', [ (('AudioInput', 'get_audioinput_bin'), gst.Bin), (('AudioMixer', 'get_audiomixer_bin'), gst.Bin), (('VideoInput', 'get_videoinput_bin'), gst.Bin), (('VideoMixer', 'get_videomixer_bin'), gst.Bin), (('Output', 'get_output_bin'), gst.Bin) ]) def test_plugin_bin(plugin_manager, plugin_info, expected_instance): """Check that a plugin and its get method returns the proper gst.Bin object""" plugin_name, get_plugin_method = plugin_info plugins = plugin_manager.get_plugins_of_category(plugin_name) for plugin in plugins: plugin.plugin_object.load_config(plugin_manager) # if plugin.name == "Firewire Source": # There is an issue with Firewire Source in testing found in gh#589 # Skip until this is resolved # continue > plugin_bin = getattr(plugin.plugin_object, get_plugin_method)() freeseer/tests/framework/test_plugins.py:59: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = <yapsy_loaded_plugin_Firewire_Source_19.FirewireSrc object at 0x7f04600dce90> def get_videoinput_bin(self): bin = gst.Bin() # Do not pass a name so that we can load this input more than once. videosrc = gst.element_factory_make("dv1394src", "videosrc") dv1394q1 = gst.element_factory_make('queue', 'dv1394q1') dv1394dvdemux = gst.element_factory_make('dvdemux', 'dv1394dvdemux') dv1394q2 = gst.element_factory_make('queue', 'dv1394q2') dv1394dvdec = gst.element_factory_make('dvdec', 'dv1394dvdec') # Add Elements bin.add(videosrc) bin.add(dv1394q1) bin.add(dv1394dvdemux) bin.add(dv1394q2) bin.add(dv1394dvdec) # Link Elements videosrc.link(dv1394q1) dv1394q1.link(dv1394dvdemux) > dv1394dvdemux.link(dv1394q2) E LinkError: failed to link dv1394dvdemux with dv1394q2 freeseer/plugins/videoinput/firewiresrc/__init__.py:100: LinkError ------------------------------- Captured stderr -------------------------------- 2014-11-03 16:52:49,449 ( DEBUG) freeseer.framework.plugin : Plugin manager initialized. --------------- coverage: platform linux2, python 2.7.6-final-0 ----------------

#644 added and updated the code comment

dideler · 2014-11-09T18:59:42Z

count_platform_plugins_in_directory looks like an expensive operation and is called quite a bit. How long does it take for these tests to run? edit: consider caching the values

SRomansky · 2014-11-10T01:03:22Z

On my laptop before caching the tests took 1.07 seconds to run, after caching 1.03 seconds. With a HDD the time difference would probably be even larger.

dideler · 2014-11-11T17:06:27Z

src/freeseer/tests/framework/test_plugins.py

+    for root, directory, files in os.walk(path, topdown=False):
+        for filename in files:
+            if filename.endswith(plugin_suffix):
+                plugin_name = os.path.splitext(filename)[0]


It's more pythonic to ignore the second element in the tuple instead of indexing.

plugin_name, _ = os.path.splitext(filename)

Oh that is a cool feature, implemented.

dideler · 2014-11-14T19:40:17Z

I think this is close to being merge-ready. Can you squash your commits?

dideler · 2014-11-18T12:47:54Z

src/freeseer/tests/framework/test_plugins.py

+    for plugin in plugins:
+        if hasattr(plugin.plugin_object, 'config'):
+            assert plugin.plugin_object.CONFIG_CLASS
+            assert not plugin.config


Is plugin.config correct or should it be plugin.plugin_object.config?

Yeah. Plugin Manager does not have a config attribute. Only IBackendPlugin has a config attribute.

However, in the unit test I replaced the not, so it was assert plugin.plugin_object.config and it passed, then I tried assert not plugin.plugin_object.config and it also passed. Will look into this later today.

It appears that the plugin_object never has a config attribute. Hence the assertions are never called. I am looking for an example of a plugin loading a plugin configuration with plugin_object.load_config(). However, freeseer/src$ grep -r load_config$.*,.*,.*$ freeseer/plugins/ shows that there are no plugins who use the config arguement on the load_config function. (It should be the third value).

That's because the backend loads configuration when it needs it. See:

https://github.com/Freeseer/freeseer/blob/master/src/freeseer/framework/multimedia.py

dideler · 2014-11-19T11:06:22Z

src/freeseer/tests/framework/test_plugins.py


 from freeseer.framework.config.profile import ProfileManager
-from freeseer.framework.plugin import PluginManager
+from freeseer.framework.plugin import PluginManager, IAudioInput
+from freeseer.framework.config import Config, options


One import per line please.

Fixed, sorry about that.

dideler · 2014-11-20T15:13:09Z

Is this good to go on your end? If so, please squash again.

SRomansky · 2014-11-20T16:14:55Z

squashed

dideler · 2014-11-20T17:32:48Z

Please fix your commit message to use proper issue referencing, see http://freeseer.readthedocs.org/en/latest/contribute/basics.html#reference-issues-in-your-commit-messages. The formatting you're using is only recognized by our IRC bot.

- Refactor test_plugins to use pytest - add new functionality tests - Add FIXME comment for firewiresrc issue - Create git issue to track FIXME status - Add a plugin information cache fixture for test efficiency Fix Freeseer#642 Related to Freeseer#619 Related to Freeseer#484

SRomansky · 2014-11-20T17:57:12Z

Fixed, sorry about that.

💄 Refactoring #642

dideler · 2014-11-20T21:08:21Z

Merged as e1d1c42, thanks @wigglier.

dideler added this to the 2014 Fall - UCOSP milestone Nov 3, 2014

dideler reviewed Nov 3, 2014
View reviewed changes

dideler reviewed Nov 11, 2014
View reviewed changes

SRomansky force-pushed the 484-plugin-test-refactor branch 2 times, most recently from 0cc13cc to 3fbdda3 Compare November 15, 2014 18:10

dideler mentioned this pull request Nov 17, 2014

Unit tests under Windows partly fail #647

Open

dideler reviewed Nov 18, 2014
View reviewed changes

dideler reviewed Nov 19, 2014
View reviewed changes

SRomansky force-pushed the 484-plugin-test-refactor branch from c9c9ada to 24c7727 Compare November 20, 2014 16:14

SRomansky force-pushed the 484-plugin-test-refactor branch from 24c7727 to b4bceb2 Compare November 20, 2014 17:57

dideler closed this in e1d1c42 Nov 20, 2014

dideler added a commit that referenced this pull request Nov 20, 2014

Minor improvements to framework/test_plugins.py

42a264c

💄 Refactoring #642

SRomansky deleted the 484-plugin-test-refactor branch November 21, 2014 22:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor and add tests for plugins.py #642

Refactor and add tests for plugins.py #642

SRomansky commented Nov 2, 2014

dideler Nov 3, 2014

zxiiro Nov 3, 2014

SRomansky Nov 3, 2014

SRomansky Nov 3, 2014

SRomansky Nov 4, 2014

dideler commented Nov 9, 2014

SRomansky commented Nov 10, 2014

dideler Nov 11, 2014

SRomansky Nov 12, 2014

dideler commented Nov 14, 2014

dideler Nov 18, 2014

SRomansky Nov 18, 2014

SRomansky Nov 18, 2014

zxiiro Nov 18, 2014

dideler Nov 19, 2014

SRomansky Nov 19, 2014

dideler commented Nov 20, 2014

SRomansky commented Nov 20, 2014

dideler commented Nov 20, 2014

SRomansky commented Nov 20, 2014

dideler commented Nov 20, 2014

Refactor and add tests for plugins.py #642

Refactor and add tests for plugins.py #642

Conversation

SRomansky commented Nov 2, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dideler commented Nov 9, 2014

SRomansky commented Nov 10, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dideler commented Nov 14, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dideler commented Nov 20, 2014

SRomansky commented Nov 20, 2014

dideler commented Nov 20, 2014

SRomansky commented Nov 20, 2014

dideler commented Nov 20, 2014