suppress react test warnings

[pharingee]

Resolves #2769

Overall change:
Add a function to suppress predicted prop warnings during tests.

Code changes:

• Create suppressPropWarnings function to add expected warnings to a global variable
• Update setupTests to ignore warnings that match expected warnings from global variable and reset global variable.
• Add suppressPropWarnings to Image tests

---

• I have assigned myself to this PR and the corresponding issues
• Tests added for new features
• Test engineer approval

[jroebu14]

I think this issue could be an opportunity to clear out some tech debt rather than suppress warnings.

If I was to approach this I would change the image props types file to be more explicit like promo's prop types and to not use this function because I think it just sets just check prop types and throws errors if they are incorrect which is what the prop-types does anyway. Would be worth checking with the author of arrayOfSpecificBlocks first though. I personally think arrayOfSpecificBlocks should be deprecated and removed.

example of more explicit prop types:

Image.propTypes = {
  blocks: arrayOf(any), // <-- this could be more detailed but I haven't check what it should be
  position: arrayOf(number),
};

Image.defaultProps = {
  blocks: [],
  position: [1],
};

the image component only accepts 2 props and by looking at the logic in the component, both are not required so you can give them default props.

You will then need to change the check that renders null if no blocks data is passed to the component:

  if (!blocks) {
    return null;
  }

would then become

  if (!blocks.length) {
    return null;
  }

I tried this out locally and tests now pass with no warnings.

[pharingee]

@jroebu14 Adding blocks to the default props prevents the warning from all areas. I think the main idea is to issue a warning when blocks isn't provided. By providing a default prop devs won't get a warning while working on the app. This can make it difficult to debug. @dr3 am I wrong in this assumption? If I am then adding blocks to default works very well.

Regarding the actual props description, using any is very dangerous as you indicated. @jroebu14 can you suggest a proper prop-type from the schema?

[dr3]

Hmmm i'm in 2 minds, part of me doesn't like setting defaults to please the tests, but also, writing defensive components is kinda nice too. Im not sure. I do think there is always going to be cases where you need to violate prop warnings in tests, so a helper like this is needed. But also, in some cases, perhaps code changes will cover it, and make our components better.

[jroebu14]

@dr3 @pharingee it wasn't really to please tests but was more tailoring prop types to match the component's API.

blocks is optional because you if you don't pass it in then the component will happily return null instead of throwing an error. If we want the developer to be aware of blocks not being provided then perhaps we should let the component throw an error and in the test change it to check that the component throws if no blocks data is passed in.

It's also possible that if we keep it the way it is then the component can essentially silently fail i.e. not render an image.

[dr3]

blocks is optional because you if you don't pass it in then the component will happily return null instead of throwing an error

In that case, isRequired should be removed, as it is inaccurate. There are cases still though where you will want to pass a propm value in a test, which doesnt match the expected

[dr3]

this currently doesnt allow you to supress 2 prop warnings at a time. E.g. below ImageContainer has 2 required props, blocks and foobar

describe('with no data', () => {
    suppressPropWarnings(['blocks', 'ImageContainer']);
    suppressPropWarnings(['foobar', 'ImageContainer']);
    isNull('should return null', <ImageContainer />);
  });

It will only supress 1 of the 2 console errors

[pharingee]

We wouldn't be able to test for that because once an error is thrown the rest of the code isn't executed. In cases where multiple props absences are detected, only one console error will be thrown so we can catch them all like this:

describe('with no data', () => {
    suppressPropWarnings(['blocks', 'foo', 'bar', 'ImageContainer']);
    isNull('should return null', <ImageContainer />);
  });

[dr3]

once an error is thrown the rest of the code isn't executed

Yeah it is, you can have 2 prop warning errors happen in 1 component.

[dr3]

is from

describe('Image', () => {
  describe('with no data', () => {
    isNull('should return null', <ImageContainer />);
  });
});

when ImageContainer has

ImageContainer.propTypes = {
  foobar: string.isRequired,
  barfoo: string.isRequired,
};

[chris-hinds]

happy with the technical approach here.