mozilla / tippy-top-sites-deprecated

[deprecated][unmaintained]
6 stars 7 forks source link

Add tests for unused images? #57

Closed pdehaan closed 7 years ago

pdehaan commented 7 years ago

Ref: https://github.com/mozilla/tippy-top-sites/pull/55#issuecomment-273881737

In our last PR, it looks like about 11 unused images may have snuck in. I [poorly] wrote a quick script to check for unused images, but we should probably have a more formal unit test or something:

const { readdir } = require('fs');

const topSites = require('./top_sites.json');

readdir('images', (err, images) => {
  if (err) throw err;

  const imageMap = new Map();
  topSites.forEach((site) => imageMap.set(site.image_url, site));

  const extraneous = images.filter((image) => !imageMap.has(image));
  console.error(`Extraneous images:\n${JSON.stringify(extraneous, null, 2)}`);
});

Output:

$ git rev-parse --short HEAD # 0e6a518

$ node extra

Extraneous images:
[
  "baidu-com-1.png",
  "cbc-ca.png",
  "duckduckgo-com.png",
  "espn.go-com.png",
  "github-com-1.png",
  "google-vault.png",
  "jcpenny-com.png",
  "jd-com.png",
  "moz-china.png",
  "qq-com.png",
  "sina-com.png",
  "taobao-com.png",
  "thedailybeast-com.png"
]

Oddly, that list has 13 results, and not 11 (introduced in last PR), so we may have had a couple existing unlinked images that we're shipping w/ tippy-top-sites, but not actually using (the offenders being "cbc-ca.png" and "duckduckgo-com.png")

pdehaan commented 7 years ago

TL;DR: It's about 66 KB of unused images.

const { readdir, statSync } = require('fs');
const { join } = require('path');

const topSites = require('./top_sites.json');

const btoKB = (bytes) => (bytes / 1024).toFixed(1);
const getSize = (image) => statSync(image).size;

readdir('images', (err, images) => {
  if (err) throw err;

  const imageMap = new Map();
  topSites.forEach((site) => imageMap.set(site.image_url, site));

  const extraneous = images.filter((image) => !imageMap.has(image))
    .map((image) => {
      return {
        image,
        size: getSize(join('images', image))
      };
    });

  const totalSize = extraneous.reduce((prev, curr) => prev += curr.size, 0);
  const res = extraneous.map(({image, size}) => `[${btoKB(size)} KB]: ${image}`);
  console.error(`Extraneous images (${btoKB(totalSize)} KB):\n${res.join('\n')}`);
});

/*
Extraneous images (66.5 KB):
[2.9 KB]: images/baidu-com-1.png
[2.9 KB]: images/cbc-ca.png
[13.5 KB]: images/duckduckgo-com.png
[1.2 KB]: images/espn.go-com.png
[2.9 KB]: images/github-com-1.png
[6.4 KB]: images/google-vault.png
[2.1 KB]: images/jcpenny-com.png
[14.4 KB]: images/jd-com.png
[4.1 KB]: images/moz-china.png
[3.2 KB]: images/qq-com.png
[3.1 KB]: images/sina-com.png
[3.2 KB]: images/taobao-com.png
[6.6 KB]: images/thedailybeast-com.png
*/
k88hudson commented 7 years ago

Yes please!

pdehaan commented 7 years ago

Possibly this piece of half-assery:

describe("extraneous", () => {
  it("images should not exist", () => {
    const imageMap = new Map();
    data.forEach((site) => imageMap.set(site.image_url, site));
    const extraneous = fs.readdirSync('images').filter((image) => !imageMap.has(image));
    assert.lengthOf(extraneous, 0);
  });
});
  extraneous
    1) images should not exist

  1370 passing (342ms)
  1 failing

  1) extraneous images should not exist:
     AssertionError: expected [ 'cbc-ca.png', 'duckduckgo-com.png' ] to have a length of 0 but got 2
      at Function.assert.lengthOf (node_modules/chai/lib/chai/interface/assert.js:1086:37)
      at Context.it (test/index.test.js:105:12)

npm ERR! Test failed.  See above for more details.

(against name-change-and-image-purge branch)

pdehaan commented 7 years ago

Fixed via #63