nature-of-code / noc-book-2

Nature of Code with p5.js and Notion workflow / build system.
https://natureofcode.com
820 stars 56 forks source link

Test to validate screenshot dimensions #519

Closed fturmel closed 8 months ago

fturmel commented 8 months ago

As discussed in #412:

Screenshot 2023-10-08 at 2 59 33 PM
netlify[bot] commented 8 months ago

Deploy Preview for nature-of-code-2nd-edition ready!

Name Link
Latest commit 6cf28f7312d4da0548e232a8887c7a05e716204e
Latest deploy log https://app.netlify.com/sites/nature-of-code-2nd-edition/deploys/65240af17a2ee30008aafa15
Deploy Preview https://deploy-preview-519--nature-of-code-2nd-edition.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 8 months ago

Deploy Preview for nature-of-code-pdf ready!

Name Link
Latest commit 6cf28f7312d4da0548e232a8887c7a05e716204e
Latest deploy log https://app.netlify.com/sites/nature-of-code-pdf/deploys/65240af1c47a590008f17af1
Deploy Preview https://deploy-preview-519--nature-of-code-pdf.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

jasongao97 commented 8 months ago

This is working great! Thank you. I would like to point out that sometimes the screenshot is not the size of the canvas. (e.g. Figure 0.2: combination of multiple shots at different times) We might just ignore those warnings.

screenshot

shiffman commented 8 months ago

Thank you @fturmel! I'm happy to merge this, let me know if it's ready or if you prefer to do any final changes. We could potentially maintain a list of "exceptions" if that would help!

fturmel commented 8 months ago

@jasongao97 Right, good point. There are 3 images that don't share the same aspect ratio as the sketch:

Maybe we can add some logic related to aspect ratio mismatch? Or simply have an ignore list? If so, it's probably a good idea to at least have a width/height baseline so that the images are high resolution enough.

shiffman commented 8 months ago

One idea would be to put a comment maybe in index.html that follows a specific syntax and includes the expected screenshot.png dimensions if it's an exception?

fturmel commented 8 months ago

Here's what I would propose, basically just a list of exceptions with their expected dimensions. It makes more sense to keep all this in one place imo.

const exceptions = new Map([
  ['figure_4_8_image', [1280, 480]],
  ['figure_4_8_circles', [1280, 480]],
  ['figure_i_2_bell_curve', [1440, 480]],
]);

describe('Validate examples screenshots`', () => {
  const createCanvasRegex = /\s*createCanvas\(\s*(\d+)\s*,\s*(\d+)/;

  glob.sync('content/examples/*/*/').forEach((exampleDir) => {
    describe(`${exampleDir} | ${editorUrls.get(exampleDir)}`, () => {
      test('`screenshot.png` should exist and be @2x compared to the p5 sketch canvas', async () => {
        const screenshotPath = path.join(exampleDir, 'screenshot.png');
        expect(fs.existsSync(screenshotPath)).toBe(true);

        let expectedWidth;
        let expectedHeight;
        const dirName = path.basename(exampleDir);

        if (exceptions.has(dirName)) {
          // use the dimensions from the `exceptions` map
          [expectedWidth, expectedHeight] = exceptions.get(dirName);
        } else {
          // extract the sketch canvas dimensions from the `createCanvas()` function call in `sketch.js`
          const sketchFilePath = path.join(exampleDir, 'sketch.js');
          const sketchFile = fs.readFileSync(sketchFilePath).toString();
          const canvasSize = createCanvasRegex.exec(sketchFile);
          expectedWidth = Number(canvasSize[1]) * 2;
          expectedHeight = Number(canvasSize[2]) * 2;
        }

        // get the actual dimensions of the screenshot
        const { width, height } = await sharp(screenshotPath).metadata();

        expect(`${width}x${height}`).toEqual(
          `${expectedWidth}x${expectedHeight}`,
        );
      });
    });
  });
});
shiffman commented 8 months ago

Yes, that's perfect!

fturmel commented 8 months ago

@shiffman great, PR updated

shiffman commented 8 months ago

Wonderful, merging! We can add more exceptions as we find them.