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
=== modified file 'src/thumbnailer.cpp'
--- src/thumbnailer.cpp 2016-02-26 02:14:30 +0000
+++ src/thumbnailer.cpp 2016-02-29 04:29:03 +0000
@@ -558,10 +558,9 @@
558 {558 {
559 if (image_extractor_)559 if (image_extractor_)
560 {560 {
561 // The image data has been extracted via vs-thumb. Update image_data in case read() throws.561 // The image data has been extracted via vs-thumb. Update image_data policy in case read() throws.
562 image_data.cache_policy = CachePolicy::cache_fullsize;562 image_data.cache_policy = CachePolicy::cache_fullsize;
563 return ImageData(Image(image_extractor_->read()), CachePolicy::cache_fullsize, Location::local);563 return ImageData(Image(image_extractor_->read()), CachePolicy::cache_fullsize, Location::local);
564 // TODO: Need to add to full-size cache here. See bug 1540753
565 }564 }
566565
567 string content_type = get_mimetype(filename_);566 string content_type = get_mimetype(filename_);
568567
=== modified file 'tests/thumbnailer-admin/thumbnailer-admin_test.cpp'
--- tests/thumbnailer-admin/thumbnailer-admin_test.cpp 2016-02-26 02:14:30 +0000
+++ tests/thumbnailer-admin/thumbnailer-admin_test.cpp 2016-02-29 04:29:03 +0000
@@ -302,11 +302,11 @@
302 AdminRunner ar;302 AdminRunner ar;
303303
304 // Put something in the cache.304 // Put something in the cache.
305 EXPECT_EQ(0, ar.run(QStringList{"get", TESTDATADIR "/testsong.ogg"}));305 EXPECT_EQ(0, ar.run(QStringList{"get", TESTDATADIR "/testvideo.ogg"}));
306 // Again, so we get a hit on thumbnail cache.306 // Again, so we get a hit on thumbnail cache.
307 EXPECT_EQ(0, ar.run(QStringList{"get", TESTDATADIR "/testsong.ogg"}));307 EXPECT_EQ(0, ar.run(QStringList{"get", TESTDATADIR "/testvideo.ogg"}));
308 // Again, with different size, so we get a hit on full-size cache.308 // Again, with different size, so we get a hit on full-size cache.
309 EXPECT_EQ(0, ar.run(QStringList{"get", TESTDATADIR "/testsong.ogg", "--size=20x20"}));309 EXPECT_EQ(0, ar.run(QStringList{"get", TESTDATADIR "/testvideo.ogg", "--size=20x20"}));
310 // Put something in the failure cache.310 // Put something in the failure cache.
311 EXPECT_EQ(1, ar.run(QStringList{"get", TESTDATADIR "/empty"}));311 EXPECT_EQ(1, ar.run(QStringList{"get", TESTDATADIR "/empty"}));
312 // Again, so we get a hit on the failure cache.312 // Again, so we get a hit on the failure cache.
@@ -316,9 +316,7 @@
316316
317 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));317 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));
318 auto output = ar.stdout();318 auto output = ar.stdout();
319 // TODO: broken, see bug 1540753319 EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
320 //EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
321 EXPECT_TRUE(output.find("Size: 0") != string::npos) << output;
322320
323 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));321 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));
324 output = ar.stdout();322 output = ar.stdout();
@@ -332,9 +330,7 @@
332330
333 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));331 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));
334 output = ar.stdout();332 output = ar.stdout();
335 // TODO: broken, see bug 1540753333 EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
336 //EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
337 EXPECT_TRUE(output.find("Hits: 0") != string::npos) << output;
338334
339 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));335 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));
340 output = ar.stdout();336 output = ar.stdout();
@@ -350,9 +346,7 @@
350346
351 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));347 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));
352 output = ar.stdout();348 output = ar.stdout();
353 // TODO: broken, see bug 1540753349 EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
354 //EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
355 EXPECT_TRUE(output.find("Hits: 0") != string::npos) << output;
356350
357 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));351 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));
358 output = ar.stdout();352 output = ar.stdout();
@@ -383,9 +377,7 @@
383377
384 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));378 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));
385 output = ar.stdout();379 output = ar.stdout();
386 // TODO: broken, see bug 1540753380 EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
387 //EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
388 EXPECT_TRUE(output.find("Size: 0") != string::npos) << output;
389381
390 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));382 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));
391 output = ar.stdout();383 output = ar.stdout();
@@ -401,9 +393,7 @@
401393
402 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));394 EXPECT_EQ(0, ar.run(QStringList{"stats", "i"}));
403 output = ar.stdout();395 output = ar.stdout();
404 // TODO: broken, see bug 1540753396 EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
405 //EXPECT_TRUE(output.find("Size: 1") != string::npos) << output;
406 EXPECT_TRUE(output.find("Size: 0") != string::npos) << output;
407397
408 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));398 EXPECT_EQ(0, ar.run(QStringList{"stats", "t"}));
409 output = ar.stdout();399 output = ar.stdout();
410400
=== modified file 'tests/thumbnailer/thumbnailer_test.cpp'
--- tests/thumbnailer/thumbnailer_test.cpp 2016-02-26 02:14:30 +0000
+++ tests/thumbnailer/thumbnailer_test.cpp 2016-02-29 04:29:03 +0000
@@ -240,30 +240,32 @@
240 EXPECT_EQ("", request->thumbnail());240 EXPECT_EQ("", request->thumbnail());
241 }241 }
242242
243 {243 // Thumbnail a video so we get something into the full-size cache.
244 // Load a video, so we have something in the full-size cache.244 {
245 auto request = tn.get_thumbnail(TEST_VIDEO, QSize(40, 40));245 auto request = tn.get_thumbnail(TEST_VIDEO, QSize(1920, 1920));
246 ASSERT_NE(nullptr, request.get());246 ASSERT_NE(nullptr, request.get());
247247 // Video thumbnails cannot be produced immediately
248 // Video thumbnails cannot be produced immediately248 ASSERT_EQ("", request->thumbnail());
249 ASSERT_EQ("", request->thumbnail());249
250250 {
251 QSignalSpy spy(request.get(), &ThumbnailRequest::downloadFinished);251 QSignalSpy spy(request.get(), &ThumbnailRequest::downloadFinished);
252 request->download(chrono::milliseconds(15000));252 request->download(chrono::milliseconds(15000));
253 ASSERT_TRUE(spy.wait(20000));253 ASSERT_TRUE(spy.wait(20000));
254 }254 }
255255
256 {256 request = tn.get_thumbnail(TEST_VIDEO, QSize(100, 100));
257 // Load the video again at different size, so we get a hit on the full-size cache.257 ASSERT_NE(nullptr, request.get());
258 auto request = tn.get_thumbnail(TEST_VIDEO, QSize(20, 20));258
259 ASSERT_NE(nullptr, request.get());259 {
260260 QSignalSpy spy(request.get(), &ThumbnailRequest::downloadFinished);
261 // Video thumbnails cannot be produced immediately261 request->download(chrono::milliseconds(15000));
262 ASSERT_EQ("", request->thumbnail());262 ASSERT_TRUE(spy.wait(20000));
263263 }
264 QSignalSpy spy(request.get(), &ThumbnailRequest::downloadFinished);264
265 request->download(chrono::milliseconds(15000));265 auto thumb = request->thumbnail();
266 ASSERT_TRUE(spy.wait(20000));266 Image img(thumb);
267 EXPECT_EQ(100, img.width());
268 EXPECT_EQ(56, img.height());
267 }269 }
268 };270 };
269271
@@ -271,10 +273,10 @@
271273
272 // Just to show that fill_cache() does put things into the cache and the stats are as expected.274 // Just to show that fill_cache() does put things into the cache and the stats are as expected.
273 auto stats = tn.stats();275 auto stats = tn.stats();
274 EXPECT_EQ(0, stats.full_size_stats.size()); // TODO: broken, needs to be 1276 EXPECT_EQ(1, stats.full_size_stats.size());
275 EXPECT_EQ(2, stats.thumbnail_stats.size());277 EXPECT_EQ(3, stats.thumbnail_stats.size());
276 EXPECT_EQ(1, stats.failure_stats.size());278 EXPECT_EQ(1, stats.failure_stats.size());
277 EXPECT_EQ(0, stats.full_size_stats.hits()); // TODO: broken, needs to be 1279 EXPECT_EQ(0, stats.full_size_stats.hits());
278 EXPECT_EQ(1, stats.thumbnail_stats.hits());280 EXPECT_EQ(1, stats.thumbnail_stats.hits());
279 EXPECT_EQ(1, stats.failure_stats.hits());281 EXPECT_EQ(1, stats.failure_stats.hits());
280282
@@ -290,7 +292,7 @@
290 tn.clear(Thumbnailer::CacheSelector::full_size_cache);292 tn.clear(Thumbnailer::CacheSelector::full_size_cache);
291 stats = tn.stats();293 stats = tn.stats();
292 EXPECT_EQ(0, stats.full_size_stats.size());294 EXPECT_EQ(0, stats.full_size_stats.size());
293 EXPECT_EQ(2, stats.thumbnail_stats.size());295 EXPECT_EQ(3, stats.thumbnail_stats.size());
294 EXPECT_EQ(1, stats.failure_stats.size());296 EXPECT_EQ(1, stats.failure_stats.size());
295297
296 // Clear thumbnail cache only.298 // Clear thumbnail cache only.
@@ -298,7 +300,7 @@
298 fill_cache();300 fill_cache();
299 tn.clear(Thumbnailer::CacheSelector::thumbnail_cache);301 tn.clear(Thumbnailer::CacheSelector::thumbnail_cache);
300 stats = tn.stats();302 stats = tn.stats();
301 EXPECT_EQ(0, stats.full_size_stats.size()); // TODO: broken, needs to be 1303 EXPECT_EQ(1, stats.full_size_stats.size());
302 EXPECT_EQ(0, stats.thumbnail_stats.size());304 EXPECT_EQ(0, stats.thumbnail_stats.size());
303 EXPECT_EQ(1, stats.failure_stats.size());305 EXPECT_EQ(1, stats.failure_stats.size());
304306
@@ -307,8 +309,8 @@
307 fill_cache();309 fill_cache();
308 tn.clear(Thumbnailer::CacheSelector::failure_cache);310 tn.clear(Thumbnailer::CacheSelector::failure_cache);
309 stats = tn.stats();311 stats = tn.stats();
310 EXPECT_EQ(0, stats.full_size_stats.size()); // TODO: broken, needs to be 1312 EXPECT_EQ(1, stats.full_size_stats.size());
311 EXPECT_EQ(2, stats.thumbnail_stats.size());313 EXPECT_EQ(3, stats.thumbnail_stats.size());
312 EXPECT_EQ(0, stats.failure_stats.size());314 EXPECT_EQ(0, stats.failure_stats.size());
313315
314 // Clear all stats.316 // Clear all stats.
@@ -334,7 +336,7 @@
334 fill_cache();336 fill_cache();
335 tn.clear_stats(Thumbnailer::CacheSelector::thumbnail_cache);337 tn.clear_stats(Thumbnailer::CacheSelector::thumbnail_cache);
336 stats = tn.stats();338 stats = tn.stats();
337 EXPECT_EQ(0, stats.full_size_stats.size()); // TODO: broken, needs to be 1339 EXPECT_EQ(1, stats.full_size_stats.size());
338 EXPECT_EQ(0, stats.thumbnail_stats.hits());340 EXPECT_EQ(0, stats.thumbnail_stats.hits());
339 EXPECT_EQ(1, stats.failure_stats.hits());341 EXPECT_EQ(1, stats.failure_stats.hits());
340342
@@ -344,7 +346,7 @@
344 fill_cache();346 fill_cache();
345 tn.clear_stats(Thumbnailer::CacheSelector::failure_cache);347 tn.clear_stats(Thumbnailer::CacheSelector::failure_cache);
346 stats = tn.stats();348 stats = tn.stats();
347 EXPECT_EQ(0, stats.full_size_stats.size()); // TODO: broken, needs to be 1349 EXPECT_EQ(1, stats.full_size_stats.size());
348 EXPECT_EQ(1, stats.thumbnail_stats.hits());350 EXPECT_EQ(1, stats.thumbnail_stats.hits());
349 EXPECT_EQ(0, stats.failure_stats.hits());351 EXPECT_EQ(0, stats.failure_stats.hits());
350}352}

Subscribers

People subscribed via source and target branches

to all changes: