Testing framework / patterns quality improvements

[bfeister]

Description

Digging into the questions we discussed re: testing (a big one being "how can we throw an error any time a test makes a real network request") I've realized that our default behavior of having error boundaries that show thrown Errors in the DOM (swallowing what happens in the console) is the reason we've not been able to make progress here. I've added the following, but tests don't throw because the error boundary in commerce-sdk-react catches this and displays it so that the throw doesn't cause a test failure.

    nock.emitter.on('no match', (req) => {
        throw Error(
            `\n\n\nYOUR TEST IS MAKING A REAL HTTP REQUEST. THIS IS A PROBLEM! PLEASE MOCK ALL HTTP CALLS\n\n\n `
        )
    })

We should talk about the error boundary, I understand the purpose in template-retail-react-app but it's strange to swallow errors in the test scaffolding in commerce-sdk-react obviously it's a headless library, so the error shouldn't be swallowed in DOM in the library itself (or we'd have a big SSR problem on our hands). I think the error boundary itself needs to be smart and probably needs to have an option to be disabled in "testing mode" via a provider that wraps all of our tests. @kevinxh I've also found a fix for the noisy console issues for the mutation error tests we can do

nock(DEFAULT_TEST_HOST).on('error', (err) => {
    // NOTE: nock does not throw console.errors but we want to assert against
    // console errors as well as swallow them to prevent noisy console test runs

    console.error(err)
})
...(other error handlers)

Which will cause each error to throw in the console. Right now, either the error boundary causes issues -- OR -- the tests are closing with the console.errors hanging, so they create the console noise. A fix for this is to expect / await the errors and spyOn(console, 'error') which makes sure the tests don't close prior to the console.error logging

Types of Changes

• [ ] Bug fix (non-breaking change that fixes an issue)
• [ ] New feature (non-breaking change that adds functionality)
• [ ] Documentation update
• [ ] Breaking change (could cause existing functionality to not work as expected)
• [ ] Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

• Removing a public function or component or prop
• Adding a required argument to a function
• Changing the data type of a function parameter or return value
• Adding a new peer dependency to package.json

Changes

• (change1)

How to Test-Drive This PR

• (step1)

Checklists

General

• [ ] Changes are covered by test cases
• [ ] CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

• [ ] There are no changes to UI

or...

• [ ] Changes were tested with a Screen Reader (iOS VoiceOver or Android Talkback) and had no issues
• [ ] Changes comply with WCAG 2.0 guidelines levels A and AA
• [ ] Changes to common UI patterns and interactions comply with WAI-ARIA best practices

Localization

• [ ] Changes include a UI text update in the Retail React App (which requires translation)

[alexvuong]

This Pr has been sitting here for awhile. What do we want with it moving forwards?

[kevinxh]

stale PR. closed.