Thumbnailer

Merge lp:~michihenning/thumbnailer/fullsize-test-fixes into lp:thumbnailer/devel

  1. fullsize-test-fixes
  2. Merge into devel
Proposed by Michi Henning
Status: Merged
Approved by: James Henstridge
Approved revision: 348
Merged at revision: 348
Proposed branch: lp:~michihenning/thumbnailer/fullsize-test-fixes
Merge into: lp:thumbnailer/devel
Diff against target: 211 lines (+44/-53)
3 files modified
src/thumbnailer.cpp (+1/-2)
tests/thumbnailer-admin/thumbnailer-admin_test.cpp (+8/-18)
tests/thumbnailer/thumbnailer_test.cpp (+35/-33)
To merge this branch: bzr merge lp:~michihenning/thumbnailer/fullsize-test-fixes
Reviewer Review Type Date Requested Status
James Henstridge Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+287429@code.launchpad.net

Commit message

Fixes for full-size tests.

Description of the change

Fixes for full-size tests. Problem was with the tests, not the code.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:348
http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-ci/676/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-amd64-ci/380
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-armhf-ci/381
        deb: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-armhf-ci/381/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-vivid-i386-ci/381
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-xenial-amd64-ci/130
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-xenial-armhf-ci/131
        deb: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-xenial-armhf-ci/131/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/thumbnailer-devel-xenial-i386-ci/128

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/thumbnailer-devel-ci/676/rebuild

review: Approve (continuous-integration)
Revision history for this message
James Henstridge (jamesh) wrote :

Looks good. Always nice to see TODO comments removed from code.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/thumbnailer.cpp'
2--- src/thumbnailer.cpp 2016-02-26 02:14:30 +0000
3+++ src/thumbnailer.cpp 2016-02-29 04:29:03 +0000
4@@ -558,10 +558,9 @@
5 {
6 if (image_extractor_)
7 {
8- // The image data has been extracted via vs-thumb. Update image_data in case read() throws.
9+ // The image data has been extracted via vs-thumb. Update image_data policy in case read() throws.
10 image_data.cache_policy = CachePolicy::cache_fullsize;
11 return ImageData(Image(image_extractor_->read()), CachePolicy::cache_fullsize, Location::local);
12- // TODO: Need to add to full-size cache here. See bug 1540753
13 }
14
15 string content_type = get_mimetype(filename_);
16
17=== modified file 'tests/thumbnailer-admin/thumbnailer-admin_test.cpp'
18--- tests/thumbnailer-admin/thumbnailer-admin_test.cpp 2016-02-26 02:14:30 +0000
19+++ tests/thumbnailer-admin/thumbnailer-admin_test.cpp 2016-02-29 04:29:03 +0000
20@@ -302,11 +302,11 @@
21 AdminRunner ar;
22
23 // Put something in the cache.
24- EXPECT_EQ(0, ar.run(QStringList{"get", TESTDATADIR "/testsong.ogg"}));
25+ EXPECT_EQ(0, ar.run(QStringList{"get", TESTDATADIR "/testvideo.ogg"}));
26 // Again, so we get a hit on thumbnail cache.
27- EXPECT_EQ(0, ar.run(QStringList{"get", TESTDATADIR "/testsong.ogg"}));
28+ EXPECT_EQ(0, ar.run(QStringList{"get", TESTDATADIR "/testvideo.ogg"}));
29 // Again, with different size, so we get a hit on full-size cache.
30- EXPECT_EQ(0, ar.run(QStringList{"get", TESTDATADIR "/testsong.ogg", "--size=20x20"}));
31+ EXPECT_EQ(0, ar.run(QStringList{"get", TESTDATADIR "/testvideo.ogg", "--size=20x20"}));
32 // Put something in the failure cache.
33 EXPECT_EQ(1, ar.run(QStringList{"get", TESTDATADIR "/empty"}));
34 // Again, so we get a hit on the failure cache.
35@@ -316,9 +316,7 @@
36
37 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));
38 auto output = ar.stdout();
39- // TODO: broken, see bug 1540753
40- //EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
41- EXPECT_TRUE(output.find("Size: 0") != string::npos) << output;
42+ EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
43
44 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));
45 output = ar.stdout();
46@@ -332,9 +330,7 @@
47
48 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));
49 output = ar.stdout();
50- // TODO: broken, see bug 1540753
51- //EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
52- EXPECT_TRUE(output.find("Hits: 0") != string::npos) << output;
53+ EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
54
55 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));
56 output = ar.stdout();
57@@ -350,9 +346,7 @@
58
59 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));
60 output = ar.stdout();
61- // TODO: broken, see bug 1540753
62- //EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
63- EXPECT_TRUE(output.find("Hits: 0") != string::npos) << output;
64+ EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
65
66 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));
67 output = ar.stdout();
68@@ -383,9 +377,7 @@
69
70 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));
71 output = ar.stdout();
72- // TODO: broken, see bug 1540753
73- //EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
74- EXPECT_TRUE(output.find("Size: 0") != string::npos) << output;
75+ EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
76
77 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));
78 output = ar.stdout();
79@@ -401,9 +393,7 @@
80
81 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));
82 output = ar.stdout();
83- // TODO: broken, see bug 1540753
84- //EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
85- EXPECT_TRUE(output.find("Size: 0") != string::npos) << output;
86+ EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
87
88 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));
89 output = ar.stdout();
90
91=== modified file 'tests/thumbnailer/thumbnailer_test.cpp'
92--- tests/thumbnailer/thumbnailer_test.cpp 2016-02-26 02:14:30 +0000
93+++ tests/thumbnailer/thumbnailer_test.cpp 2016-02-29 04:29:03 +0000
94@@ -240,30 +240,32 @@
95 EXPECT_EQ("", request->thumbnail());
96 }
97
98- {
99- // Load a video, so we have something in the full-size cache.
100- auto request = tn.get_thumbnail(TEST_VIDEO, QSize(40, 40));
101- ASSERT_NE(nullptr, request.get());
102-
103- // Video thumbnails cannot be produced immediately
104- ASSERT_EQ("", request->thumbnail());
105-
106- QSignalSpy spy(request.get(), &ThumbnailRequest::downloadFinished);
107- request->download(chrono::milliseconds(15000));
108- ASSERT_TRUE(spy.wait(20000));
109- }
110-
111- {
112- // Load the video again at different size, so we get a hit on the full-size cache.
113- auto request = tn.get_thumbnail(TEST_VIDEO, QSize(20, 20));
114- ASSERT_NE(nullptr, request.get());
115-
116- // Video thumbnails cannot be produced immediately
117- ASSERT_EQ("", request->thumbnail());
118-
119- QSignalSpy spy(request.get(), &ThumbnailRequest::downloadFinished);
120- request->download(chrono::milliseconds(15000));
121- ASSERT_TRUE(spy.wait(20000));
122+ // Thumbnail a video so we get something into the full-size cache.
123+ {
124+ auto request = tn.get_thumbnail(TEST_VIDEO, QSize(1920, 1920));
125+ ASSERT_NE(nullptr, request.get());
126+ // Video thumbnails cannot be produced immediately
127+ ASSERT_EQ("", request->thumbnail());
128+
129+ {
130+ QSignalSpy spy(request.get(), &ThumbnailRequest::downloadFinished);
131+ request->download(chrono::milliseconds(15000));
132+ ASSERT_TRUE(spy.wait(20000));
133+ }
134+
135+ request = tn.get_thumbnail(TEST_VIDEO, QSize(100, 100));
136+ ASSERT_NE(nullptr, request.get());
137+
138+ {
139+ QSignalSpy spy(request.get(), &ThumbnailRequest::downloadFinished);
140+ request->download(chrono::milliseconds(15000));
141+ ASSERT_TRUE(spy.wait(20000));
142+ }
143+
144+ auto thumb = request->thumbnail();
145+ Image img(thumb);
146+ EXPECT_EQ(100, img.width());
147+ EXPECT_EQ(56, img.height());
148 }
149 };
150
151@@ -271,10 +273,10 @@
152
153 // Just to show that fill_cache() does put things into the cache and the stats are as expected.
154 auto stats = tn.stats();
155- EXPECT_EQ(0, stats.full_size_stats.size()); // TODO: broken, needs to be 1
156- EXPECT_EQ(2, stats.thumbnail_stats.size());
157+ EXPECT_EQ(1, stats.full_size_stats.size());
158+ EXPECT_EQ(3, stats.thumbnail_stats.size());
159 EXPECT_EQ(1, stats.failure_stats.size());
160- EXPECT_EQ(0, stats.full_size_stats.hits()); // TODO: broken, needs to be 1
161+ EXPECT_EQ(0, stats.full_size_stats.hits());
162 EXPECT_EQ(1, stats.thumbnail_stats.hits());
163 EXPECT_EQ(1, stats.failure_stats.hits());
164
165@@ -290,7 +292,7 @@
166 tn.clear(Thumbnailer::CacheSelector::full_size_cache);
167 stats = tn.stats();
168 EXPECT_EQ(0, stats.full_size_stats.size());
169- EXPECT_EQ(2, stats.thumbnail_stats.size());
170+ EXPECT_EQ(3, stats.thumbnail_stats.size());
171 EXPECT_EQ(1, stats.failure_stats.size());
172
173 // Clear thumbnail cache only.
174@@ -298,7 +300,7 @@
175 fill_cache();
176 tn.clear(Thumbnailer::CacheSelector::thumbnail_cache);
177 stats = tn.stats();
178- EXPECT_EQ(0, stats.full_size_stats.size()); // TODO: broken, needs to be 1
179+ EXPECT_EQ(1, stats.full_size_stats.size());
180 EXPECT_EQ(0, stats.thumbnail_stats.size());
181 EXPECT_EQ(1, stats.failure_stats.size());
182
183@@ -307,8 +309,8 @@
184 fill_cache();
185 tn.clear(Thumbnailer::CacheSelector::failure_cache);
186 stats = tn.stats();
187- EXPECT_EQ(0, stats.full_size_stats.size()); // TODO: broken, needs to be 1
188- EXPECT_EQ(2, stats.thumbnail_stats.size());
189+ EXPECT_EQ(1, stats.full_size_stats.size());
190+ EXPECT_EQ(3, stats.thumbnail_stats.size());
191 EXPECT_EQ(0, stats.failure_stats.size());
192
193 // Clear all stats.
194@@ -334,7 +336,7 @@
195 fill_cache();
196 tn.clear_stats(Thumbnailer::CacheSelector::thumbnail_cache);
197 stats = tn.stats();
198- EXPECT_EQ(0, stats.full_size_stats.size()); // TODO: broken, needs to be 1
199+ EXPECT_EQ(1, stats.full_size_stats.size());
200 EXPECT_EQ(0, stats.thumbnail_stats.hits());
201 EXPECT_EQ(1, stats.failure_stats.hits());
202
203@@ -344,7 +346,7 @@
204 fill_cache();
205 tn.clear_stats(Thumbnailer::CacheSelector::failure_cache);
206 stats = tn.stats();
207- EXPECT_EQ(0, stats.full_size_stats.size()); // TODO: broken, needs to be 1
208+ EXPECT_EQ(1, stats.full_size_stats.size());
209 EXPECT_EQ(1, stats.thumbnail_stats.hits());
210 EXPECT_EQ(0, stats.failure_stats.hits());
211 }

Subscribers

People subscribed via source and target branches

to all changes: