Make fakeAddon a helper using createInternalAddon, not an object

[tofumatt]

Using fakeAddon isn't perfect and @kumar303 pointed out we should wrap it in createInternalAddon() from core/reducers/addons, but this leads to a LOT of boilterplate in tests. See: mozilla/addons-frontend#3203 (comment)

What we should really have is a fakeAddon() helper that takes extra props, overriding the defaults in the fakeAddon object and returns the result of createInternalAddon().

[willdurand]

We started to use various createXYZ helpers, so maybe we could introduce a createFakeAddon helper like:

const createFakeAddon = (props = {}) => createInternalAddon({ ...fakeAddon, ...props });

That would ease the migration.

[bobsilverberg]

We just need to be careful that everyone understands the naming conventions. fakeAddon is an external addon, not an internal addon (as are all of the other fakeXxx objects afaik). The name createFakeAddon therefore seems a bit ambiguous to me: does it create an external addon or an internal addon? It sounds from the above comment that we should assume that any createXxx function creates an internal object, but that doesn't seem necessarily intuitive to me.

Personally, I'm happy with just having fakeXxx objects that are external objects and knowing that I can call createInternalXxx in order to transform them into internal objects if I need an internal object. I feel that results in tests that are easy to follow without having to make any assumptions about what a createFakeXxx function might be returning.

As an aside, I've also come across a few tests recently where we were using an internal object when we really should have been using an external object. This can happen especially with addon because the internal object (prior to my recent refactor) contained all of the properties of the external object. This is another reason why I think it's valuable for tests to be explicit about whether they are using internal objects or external objects.

[willdurand]

We just need to be careful that everyone understands the naming conventions. fakeAddon is an external addon, not an internal addon (as are all of the other fakeXxx objects afaik). The name createFakeAddon therefore seems a bit ambiguous to me: does it create an external addon or an internal addon? It sounds from the above comment that we should assume that any createXxx function creates an internal object, but that doesn't seem necessarily intuitive to me.

Personally, I'm happy with just having fakeXxx objects that are external objects and knowing that I can call createInternalXxx in order to transform them into internal objects if I need an internal object. I feel that results in tests that are easy to follow without having to make any assumptions about what a createFakeXxx function might be returning.

Yep, that makes sense. I like the naming conventions suggested here.

• fakeXYZ should be the raw/external objects
• createXYZ should be explicit, so if we need a way to create fake internal addons, the function should be named createInternalFakeAddon. That being said, writing createInternalAddon(fakeAddon) makes sense and is not a lot longer than the function name mentioned previously

As an aside, I've also come across a few tests recently where we were using an internal object when we really should have been using an external object.

Yep, I've seen that too..