OpenLP

Merge lp:~tomasgroth/openlp/bug719514 into lp:openlp

  1. bug719514
  2. Merge into trunk
Proposed by Tomas Groth
Status: Superseded
Proposed branch: lp:~tomasgroth/openlp/bug719514
Merge into: lp:openlp
Diff against target: 152 lines (+120/-3)
3 files modified
openlp/core/ui/thememanager.py (+1/-1)
scripts/jenkins_script.py (+4/-2)
tests/functional/openlp_core_ui/test_thememanager.py (+115/-0)
To merge this branch: bzr merge lp:~tomasgroth/openlp/bug719514
Reviewer Review Type Date Requested Status
Tim Bentley Needs Fixing
Raoul Snyman Approve
Samuel Mehrbrodt Pending
Review via email: mp+226634@code.launchpad.net

This proposal supersedes a proposal from 2014-07-07.

This proposal has been superseded by a proposal from 2014-08-04.

Description of the change

Fix bug 719514.

To post a comment you must log in.
Revision history for this message
Tomas Groth (tomasgroth) wrote : Posted in a previous version of this proposal

lp:~tomasgroth/openlp/bug719514 (revision 2396)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/500/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/458/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/403/
[SUCCESS] http://ci.openlp.org/job/Branch-04-Windows_Tests/362/
[SUCCESS] http://ci.openlp.org/job/Branch-05a-Code_Analysis/241/
[SUCCESS] http://ci.openlp.org/job/Branch-05b-Test_Coverage/115/

Revision history for this message
Samuel Mehrbrodt (sam92) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal

Some minor fixes to your tests, but otherwise looking good. Technically you also want to test the case where the files are the same (it should be simple to do if you just copy the current test and modify it where necessary)

review: Needs Fixing
Revision history for this message
Tomas Groth (tomasgroth) wrote : Posted in a previous version of this proposal

lp:~tomasgroth/openlp/bug719514 (revision 2399)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/515/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/473/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/418/
[SUCCESS] http://ci.openlp.org/job/Branch-04-Windows_Tests/377/
[SUCCESS] http://ci.openlp.org/job/Branch-05a-Code_Analysis/246/
[SUCCESS] http://ci.openlp.org/job/Branch-05b-Test_Coverage/120/

Also fixed the tag test.

Revision history for this message
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal

Hi Tomas. My apologies, I reworked the way the bzr tag test works today and how we use it in CI. Could you merge from trunk and remove the 2.0.x tags please?

review: Needs Fixing
Revision history for this message
Tomas Groth (tomasgroth) wrote :

Merged with trunk and reverted changes to the tag test and updated jenkins script to match tests.

Revision history for this message
Tim Bentley (trb143) wrote :

Please re run CI tests

review: Needs Fixing
Revision history for this message
Tomas Groth (tomasgroth) wrote :

lp:~tomasgroth/openlp/bug719514 (revision 2402)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/552/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/505/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/449/
[SUCCESS] http://ci.openlp.org/job/Branch-04a-Windows_Functional_Tests/408/
[SUCCESS] http://ci.openlp.org/job/Branch-04b-Windows_Interface_Tests/21/
[SUCCESS] http://ci.openlp.org/job/Branch-05a-Code_Analysis/261/
[FAILURE] http://ci.openlp.org/job/Branch-05b-Test_Coverage/135/

Revision history for this message
Tim Bentley (trb143) :
review: Approve
Revision history for this message
Raoul Snyman (raoul-snyman) :
review: Approve
Revision history for this message
Tim Bentley (trb143) wrote :

Needs to re roll due to conficts.
Sorry

review: Needs Fixing
lp:~tomasgroth/openlp/bug719514 updated
2403. By Tomas Groth

trunk

2404. By Tomas Groth

deleted file leftover from merge

2405. By Tomas Groth

Fixed windows test failure by removing special char from filenames

2406. By Tomas Groth

Fixed another windows test by properly constructing paths.

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openlp/core/ui/thememanager.py'
2--- openlp/core/ui/thememanager.py 2014-03-20 19:10:31 +0000
3+++ openlp/core/ui/thememanager.py 2014-07-14 07:37:05 +0000
4@@ -650,7 +650,7 @@
5 finally:
6 if out_file:
7 out_file.close()
8- if image_from and image_from != image_to:
9+ if image_from and os.path.abspath(image_from) != os.path.abspath(image_to):
10 try:
11 encoding = get_filesystem_encoding()
12 shutil.copyfile(str(image_from).encode(encoding), str(image_to).encode(encoding))
13
14=== modified file 'scripts/jenkins_script.py'
15--- scripts/jenkins_script.py 2014-05-21 15:22:49 +0000
16+++ scripts/jenkins_script.py 2014-07-14 07:37:05 +0000
17@@ -62,11 +62,13 @@
18 Branch_Pull = 'Branch-01-Pull'
19 Branch_Functional = 'Branch-02-Functional-Tests'
20 Branch_Interface = 'Branch-03-Interface-Tests'
21- Branch_Windows = 'Branch-04-Windows_Tests'
22+ Branch_Windows_Functional = 'Branch-04a-Windows_Functional_Tests'
23+ Branch_Windows_Interface = 'Branch-04b-Windows_Interface_Tests'
24 Branch_PEP = 'Branch-05a-Code_Analysis'
25 Branch_Coverage = 'Branch-05b-Test_Coverage'
26
27- Jobs = [Branch_Pull, Branch_Functional, Branch_Interface, Branch_Windows, Branch_PEP, Branch_Coverage]
28+ Jobs = [Branch_Pull, Branch_Functional, Branch_Interface, Branch_Windows_Functional, Branch_Windows_Interface,
29+ Branch_PEP, Branch_Coverage]
30
31
32 class Colour(object):
33
34=== added file 'tests/functional/openlp_core_ui/test_thememanager.py'
35--- tests/functional/openlp_core_ui/test_thememanager.py 1970-01-01 00:00:00 +0000
36+++ tests/functional/openlp_core_ui/test_thememanager.py 2014-07-14 07:37:05 +0000
37@@ -0,0 +1,115 @@
38+# -*- coding: utf-8 -*-
39+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
40+
41+###############################################################################
42+# OpenLP - Open Source Lyrics Projection #
43+# --------------------------------------------------------------------------- #
44+# Copyright (c) 2008-2014 Raoul Snyman #
45+# Portions copyright (c) 2008-2014 Tim Bentley, Gerald Britton, Jonathan #
46+# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub, #
47+# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer. #
48+# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru, #
49+# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith, #
50+# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock, #
51+# Frode Woldsund, Martin Zibricky, Patrick Zimmermann #
52+# --------------------------------------------------------------------------- #
53+# This program is free software; you can redistribute it and/or modify it #
54+# under the terms of the GNU General Public License as published by the Free #
55+# Software Foundation; version 2 of the License. #
56+# #
57+# This program is distributed in the hope that it will be useful, but WITHOUT #
58+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or #
59+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for #
60+# more details. #
61+# #
62+# You should have received a copy of the GNU General Public License along #
63+# with this program; if not, write to the Free Software Foundation, Inc., 59 #
64+# Temple Place, Suite 330, Boston, MA 02111-1307 USA #
65+###############################################################################
66+"""
67+Package to test the openlp.core.ui.slidecontroller package.
68+"""
69+import os
70+
71+from unittest import TestCase
72+
73+from openlp.core.common import Registry
74+from openlp.core.ui import ThemeManager
75+
76+from tests.utils.constants import TEST_RESOURCES_PATH
77+from tests.interfaces import MagicMock, patch
78+
79+
80+class TestThemeManager(TestCase):
81+
82+ def setUp(self):
83+ """
84+ Set up the tests
85+ """
86+ Registry.create()
87+
88+ def initial_theme_manager_test(self):
89+ """
90+ Test the instantiation of theme manager.
91+ """
92+ # GIVEN: A new service manager instance.
93+ ThemeManager(None)
94+
95+ # WHEN: the default theme manager is built.
96+ # THEN: The the controller should be registered in the registry.
97+ self.assertIsNotNone(Registry().get('theme_manager'), 'The base theme manager should be registered')
98+
99+ def write_theme_same_image_test(self):
100+ """
101+ Test that we don't try to overwrite a theme background image with itself
102+ """
103+ # GIVEN: A new theme manager instance, with mocked builtins.open, shutil.copyfile,
104+ # theme, check_directory_exists and thememanager-attributes.
105+ with patch('builtins.open') as mocked_open, \
106+ patch('openlp.core.ui.thememanager.shutil.copyfile') as mocked_copyfile, \
107+ patch('openlp.core.ui.thememanager.check_directory_exists') as mocked_check_directory_exists:
108+ mocked_open.return_value = MagicMock()
109+ theme_manager = ThemeManager(None)
110+ theme_manager.old_background_image = None
111+ theme_manager.generate_and_save_image = MagicMock()
112+ theme_manager.path = ''
113+ mocked_theme = MagicMock()
114+ mocked_theme.theme_name = 'themename'
115+ mocked_theme.extract_formatted_xml = MagicMock()
116+ mocked_theme.extract_formatted_xml.return_value = 'fake_theme_xml'.encode()
117+
118+ # WHEN: Calling _write_theme with path to the same image, but the path written slightly different
119+ file_name1 = os.path.join(TEST_RESOURCES_PATH, 'church.jpg')
120+ # Do replacement from end of string to avoid problems with path start
121+ file_name2 = file_name1[::-1].replace(os.sep, os.sep + os.sep, 2)[::-1]
122+ theme_manager._write_theme(mocked_theme, file_name1, file_name2)
123+
124+ # THEN: The mocked_copyfile should not have been called
125+ self.assertFalse(mocked_copyfile.called, 'shutil.copyfile should not be called')
126+
127+ def write_theme_diff_images_test(self):
128+ """
129+ Test that we do overwrite a theme background image when a new is submitted
130+ """
131+ # GIVEN: A new theme manager instance, with mocked builtins.open, shutil.copyfile,
132+ # theme, check_directory_exists and thememanager-attributes.
133+ with patch('builtins.open') as mocked_open, \
134+ patch('openlp.core.ui.thememanager.shutil.copyfile') as mocked_copyfile, \
135+ patch('openlp.core.ui.thememanager.check_directory_exists') as mocked_check_directory_exists:
136+ mocked_open.return_value = MagicMock()
137+ theme_manager = ThemeManager(None)
138+ theme_manager.old_background_image = None
139+ theme_manager.generate_and_save_image = MagicMock()
140+ theme_manager.path = ''
141+ mocked_theme = MagicMock()
142+ mocked_theme.theme_name = 'themename'
143+ mocked_theme.extract_formatted_xml = MagicMock()
144+ mocked_theme.extract_formatted_xml.return_value = 'fake_theme_xml'.encode()
145+
146+ # WHEN: Calling _write_theme with path to different images
147+ file_name1 = os.path.join(TEST_RESOURCES_PATH, 'church.jpg')
148+ file_name2 = os.path.join(TEST_RESOURCES_PATH, 'church2.jpg')
149+ theme_manager._write_theme(mocked_theme, file_name1, file_name2)
150+
151+ # THEN: The mocked_copyfile should not have been called
152+ self.assertTrue(mocked_copyfile.called, 'shutil.copyfile should be called')