Uploading images to S3 in Ember.js – Part 4: Testing

20 May 2022

We concluded Part 3 of this series by having added the ability to upload an image to a band both when creating and when editing one.

In this post, we'll add a couple of tests to safeguard those user scenarios.

Existing test for creating a band

In the Rock & Roll with Ember book we'd written a test to verify creating a band works fine. It looks something like this:

 1// tests/acceptance/bands-test.js
 2test('Create a band', async function (assert) {
 3  this.server.create('band', { name: 'Royal Blood' });
 4
 5  await visit('/');
 6  await createBand('Red Hot Chili Peppers');
 7  await waitFor('[data-test-rr="no-songs-text"]');
 8
 9  assert
10    .dom('[data-test-rr="band-list-item"]')
11    .exists({ count: 2 }, 'A new band link is rendered');
12  assert
13    .dom('[data-test-rr="band-list-item"]:last-child')
14    .hasText('Red Hot Chili Peppers', 'The new band link is rendered as the last item');
15});

The createBand is a custom test helper that reduces boilerplate without reducing the clarity of what's going on in the test setup (or so I hope): 

1export async function createBand(name) {
2  await click('[data-test-rr="new-band-button"]');
3  await fillIn('[data-test-rr="new-band-name"]', name);
4  return click('[data-test-rr="save-band-button"]');
5}

We need to extend the test to allow uploading an image. More precisely, we want to end up with two test cases: one where an image is uploaded and one where it's not.

A bug revealed

In my experience, even just thinking about what to test is a great way to reduce the number of bugs in your code. It forces you to consider scenarios you might have forgot about when writing the implementation (which I guess is one of the reasons test-driven development is valuable).

In our case, I'd made a colossal error – I assumed that an image was always uploaded when creating a new band.

Look at the code:

 1// app/components/band-form.js
 2export default class BandFormComponent extends Component {
 3  // (...)
 4  @action
 5  async save(event) {
 6    event.preventDefault();
 7    let response = await fetch('/presign-aws-request', {
 8      method: 'POST',
 9    });
10    let { url, url_fields: urlFields } = await response.json();
11    let bandProperties = {
12      name: this.name,
13    };
14
15    let formData = new FormData();
16    for (let field in urlFields) {
17      formData.append(field, urlFields[field]);
18    }
19    formData.append('file', this.imageToUpload);
20
21    let imageUploadResponse = await fetch(url, {
22      method: 'POST',
23      body: formData,
24    });
25
26    if (imageUploadResponse.ok) {
27      bandProperties['image-url'] = imageUploadResponse.headers.get('Location');
28      this.imageToUpload = null;
29    }
30    return await this.args.onSave(bandProperties);
31  }
32}

Nothing checks for the presence of this.imageToUpload and the very first line posts to the back-end for a pre-signed URL. This is not needed if the user hasn't uploaded the image, just as all the following code is not.

So the code should be modified as follows:

 1// app/components/band-form.js
 2export default class BandFormComponent extends Component {
 3  // (...)
 4  @action
 5  async save(event) {
 6    event.preventDefault();
 7    let bandProperties = {
 8      name: this.name,
 9    };
10
11    if (this.imageToUpload) {
12      let response = await fetch(`${ENV.apiHost}/presign-aws-request`, {
13        method: 'POST',
14      });
15      let { url, url_fields: urlFields } = await response.json();
16
17      let formData = new FormData();
18      for (let field in urlFields) {
19        formData.append(field, urlFields[field]);
20      }
21      formData.append('file', this.imageToUpload);
22
23      let imageUploadResponse = await fetch(url, {
24        method: 'POST',
25        body: formData,
26      });
27
28      if (imageUploadResponse.ok) {
29        bandProperties['image-url'] =
30          imageUploadResponse.headers.get('Location');
31        this.imageToUpload = null;
32      }
33    }
34
35    return await this.args.onSave(bandProperties);
36  }
37}

I'm following the path I trod when adding the tests - I realized the above bug before even starting to write the tests. As the next step, let's write the tests as I did.

Writing new tests

We want at least two test scenarios for band creation: one where an image is uploaded and one where it's not. If you take a look at the existing test, it makes use of a custom test helper called createBand which hides the details about the needed user actions:

1// tests/helpers/custom-helpers.js
2import { click, fillIn } from '@ember/test-helpers';
3
4export async function createBand(name) {
5  await click('[data-test-rr="new-band-button"]');
6  await fillIn('[data-test-rr="new-band-name"]', name);
7  return click('[data-test-rr="save-band-button"]');
8}

We can extend this helper to take an optional image argument and then select that file when it's passed.

Selecting a file is done by triggering a change event on the file upload input, passing along the selected files:

 1// tests/helpers/custom-helpers.js
 2import { click, fillIn, triggerEvent } from '@ember/test-helpers';
 3
 4export async function createBand({ name, image }) {
 5  await click('[data-test-rr="new-band-button"]');
 6  await fillIn('[data-test-rr="new-band-name"]', name);
 7  if (image) {
 8    await triggerEvent('[name="file-upload"]', 'change', { files: [image] });
 9  }
10  return click('[data-test-rr="save-band-button"]');
11}

In the process, we also switched to named arguments.

Let's now write the test scenarios we've imagined.

Test case for creating a band with an image

 1// tests/acceptance/bands-test.js
 2// (...)
 3import { createBand } from 'rarwe/tests/helpers/custom-helpers';
 4
 5module('Acceptance | bands', function (hooks) {
 6  // (...)
 7  test('Create a band — with image', async function (assert) {
 8    this.server.create('band', { name: 'Royal Blood' });
 9
10    await visit('/');
11    let image = new File([], 'red-hot-chilli-peppers.jpg', {
12      type: 'image/jpeg',
13    });
14    await createBand({
15      name: 'Red Hot Chili Peppers',
16      image,
17    });
18    await waitFor('[data-test-rr="band-image"]');
19
20    assert.dom('[data-test-rr="band-image"]').exists('The band image is shown');
21    assert
22      .dom('[data-test-rr="band-list-item"]')
23      .exists({ count: 2 }, 'A new band link is rendered');
24    assert
25      .dom('[data-test-rr="band-list-item"]:last-child')
26      .hasText(
27        'Red Hot Chili Peppers',
28        'The new band link is rendered as the last item'
29      );
30    assert
31      .dom('[data-test-rr="details-nav-item"] > .active')
32      .exists('The Details tab is active');
33  });
34});

A File object is what's passed "in the real app" so we create one manually in the tests. The reason we don't have to pass its contents (see the empty array passed as the first argument to the constructor) is that the endpoints are mocked out in the test anyways, so we'll pretend we got the right thing.

We add a waitFor to make sure the transition to the Details page of the newly created band is complete.

The test will fail as we haven't added Mirage handlers for the presign back-end endpoint and the AWS bucket URL. Let's open up mirage/config.js and add them.

 1// mirage/config.js
 2import { Response } from 'miragejs';
 3
 4const s3BucketUrl = 'https://rarwe-dev.s3.eu-west-1.amazonaws.com';
 5const bucketName = 'rarwe-dev';
 6const imageId = '69c4abfc-e900-4ec7-9263-c2506bdd4c19';
 7const keyInBucket = `image-uploads/${imageId}/`;
 8const imageFileName = 'red-hot-chilli-peppers.jpg';
 9
10export default function () {
11  // (...)
12  this.post('/presign-aws-request', function () {
13    return new Response(
14      200,
15      {},
16      {
17        url: s3BucketUrl,
18        url_fields: {
19          key: keyInBucket + '${filename}',
20          success_action_status: '201',
21          policy: '...', // the actual, serialized bucket policy
22        },
23      }
24    );
25  });
26
27  this.post(s3BucketUrl, function (schema, request) {
28    let key = request.requestBody
29      .get('key')
30      .replace('${filename}', imageFileName);
31
32    let location = `${s3BucketUrl}/${encodeURIComponent(key)}`;
33    return new Response(
34      201,
35      {
36        Location: location,
37      },
38      `<PostResponse>
39        <Location>${location}</Location>
40        <Bucket>${bucketName}</Bucket>
41        <Key>${keyInBucket}</Key>
42        <ETag>"0af669d3f4786c05d779a0a7d44f6c61"</ETag>
43      </PostResponse>
44      `
45    );
46  });
47}

I took the properties of an image that had been uploaded and used them as properties of the mock response, mimicking how the real endpoints work. The most important bit is the Location header of the AWS response since that's going to be set as the image-url property of the band.

After we've added the above handlers, the test passes fine:

Test for creating band with uploaded image

Thinking about lying tests

Since we're testing against a mock back-end, let's pause for a moment to think through what exactly we are testing, or, more importantly, how could the test give us a false positive or negative.

How could the test give us false security? We always set the same image URL in the Location header and thus the same image-url will be set for the band, we don't know if the actual image that was uploaded will be properly assigned to the band object. If we were to introduce a bug to the Ember app which modifies the image-url somehow we wouldn't know because the Location is still sent back correctly.

However, if you look at the code again, you'll find that the Ember app doesn't alter any part of that URL, so that's not probable. This is done by our back-end (and S3) and should be tested there.

 1// app/components/band-form.js
 2export default class BandFormComponent extends Component {
 3  // (...)
 4  @action
 5  async save(event) {
 6    event.preventDefault();
 7    let bandProperties = {
 8      name: this.name,
 9    };
10
11    if (this.imageToUpload) {
12      let response = await fetch(`${ENV.apiHost}/presign-aws-request`, {
13        method: 'POST',
14      });
15      let { url, url_fields: urlFields } = await response.json();
16
17      let formData = new FormData();
18      for (let field in urlFields) {
19        formData.append(field, urlFields[field]);
20      }
21      formData.append('file', this.imageToUpload);
22
23      let imageUploadResponse = await fetch(url, {
24        method: 'POST',
25        body: formData,
26      });
27
28      if (imageUploadResponse.ok) {
29        bandProperties['image-url'] =
30          imageUploadResponse.headers.get('Location');
31        this.imageToUpload = null;
32      }
33    }
34
35    return await this.args.onSave(bandProperties);
36  }
37}

A second, important scenario we're not currently testing is whether the image is too big to upload. Since that's done on the client-side, this has nothing to do with mocking, though, and we could test it by passing in "bytes" as the first argument of new File.

Let's now think about false negatives, the situation in which the test fails even though the feature works fine.

The only thing I can think of as a "false negative candidate" is the deletion of the hard-wired image from the S3 bucket. However, even though the image wouldn't display, the image-url would still be set and thus the assert.dom('[data-test-rr="band-image"]').exists() assertion would still pass.

With all of the above said, in "real" apps it's crucial to have end-to-end tests that don't switch out any of the architectural pieces so as to minimize the chance of tests not telling the truth.

Let's wrap up by writing a couple of tests for scenarios we haven't tested yet.

Test case for creating a band without an image

The bug I realized the app when I started to think about tests occurred when no image was uploaded for the band.

Let's write the test for this occurrence so that we don't regress on it:

 1// tests/acceptance/bands-test.js
 2// (...)
 3import { createBand } from 'rarwe/tests/helpers/custom-helpers';
 4
 5module('Acceptance | bands', function (hooks) {
 6  // (...)
 7  test('Create a band — without image', async function (assert) {
 8    this.server.create('band', { name: 'Royal Blood' });
 9
10    await visit('/');
11    await createBand({ name: 'Caspian' });
12
13    await waitFor('[data-test-rr="no-songs-text"]');
14
15    assert
16      .dom('[data-test-rr="band-list-item"]')
17      .exists({ count: 2 }, 'A new band link is rendered');
18    assert
19      .dom('[data-test-rr="band-list-item"]:last-child')
20      .hasText('Caspian', 'The new band link is rendered as the last item');
21    assert
22      .dom('[data-test-rr="songs-nav-item"] > .active')
23      .exists('The Songs tab is active');
24  });
25});

Test case for adding an image to an existing band

The other possibility to upload an image for a band is after creation, through editing the band:

 1// tests/acceptance/bands-test.js
 2// (...)
 3import { createBand } from 'rarwe/tests/helpers/custom-helpers';
 4
 5module('Acceptance | bands', function (hooks) {
 6  // (...)
 7  test('Add image to band', async function (assert) {
 8    this.server.create('band', { name: 'Red Hot Chili Peppers' });
 9
10    let image = new File([], 'red-hot-chilli-peppers.jpg', {
11      type: 'image/jpeg',
12    });
13
14    await visit('/');
15    await click('[data-test-rr="band-link"]');
16    await click('[data-test-rr="details-nav-item"] > a');
17    await click('[data-test-rr="edit-band-link"]');
18    await triggerEvent('[name="file-upload"]', 'change', { files: [image] });
19    await click('[data-test-rr="save-band-button"]');
20
21    assert.dom('[data-test-rr="band-image"]').exists('The band image is shown');
22    assert
23      .dom('[data-test-rr="details-nav-item"] > .active')
24      .exists('The Details tab is active');
25  });
26});

Why do we need the waitFor calls?

Something that bothered me is that in several testing scenarios we needed to insert waitFor calls before we could assert the presence of DOM elements.

waitFors should only be needed in cases where Ember's testing framework can't figure it out on its own that it still needs to wait for some async operation. This could be the case when we integrate a 3rd-party library (like rendering a D3 graph) but it didn't seem like this was the case here.

It then occurred to me that when I save the band, I call await this.catalog.create('band', properties). This is a custom action in the catalog service I wrote and it calls fetch in its body. However, it doesn't use the fetch exported by ember-fetch which means the fetch is not run-loop aware and so Ember's test runner will not know to wait for it. The test runner moves on to the next line (which is usually an assertion) before the catalog.create could finish.

And indeed, simply putting the import fetch from 'fetch' as the first line in the catalog service makes the waitFor calls unnecessary.

Cool, we now have a decent test coverage for the image upload scenarios.

Share on Twitter