chore: add comprehensive testing guidelines #99

cryptotavares · 2024-07-11T12:00:08Z

Description

This PR introduces a documentation outlining MetaMask's approach to testing, including unit, integration, and end-to-end (e2e) tests. This document provides aims to provide contributors with a clear understanding of MetaMask's testing practices, encouraging comprehensive test coverage and contributing to the robustness of the application.

Changes

Added test_guidelines.md to the docs folder in the contributor docs GitHub repo.
The document includes sections on integration tests, unit tests, and e2e tests, each detailing the purpose, guidelines, and recommended practices for these types of tests.

hjetpoluru · 2024-07-11T19:13:43Z

Thanks @cryptotavares, for documenting the guidelines. I noticed that you have started with the integration test in this PR. Could you please provide more details on the integration tests themselves? It would be helpful to have a clearer understanding of the expectations, along with some examples.

MajorLift

I have a few questions about the overall scope of the doc:

Is this doc intended to only cover UI testing? The references to "page-level components" make it difficult to apply this doc to any testing that doesn't involve the presentation layer. For example, is the section on integration testing relevant to how we would test interactions between multiple MetaMask controllers?

Also, I'm a little confused as to what this doc is trying to achieve. If we are providing additional insights about each type of tests, shouldn't those insights be added to the testing docs themselves? Are we essentially providing a consolidated table of contents for our testing docs? Or are we trying to convey insights that encompass multiple types of tests and therefore don't belong in any single testing doc?

MajorLift · 2024-07-12T15:23:56Z

docs/test-guidelines.md

+
+## What are unit tests?
+
+Unit tests understanding is a bit more diffuse. As there is no consensus on how they should be written and what they should test. But generically, [as captured by Martin Fowler](https://martinfowler.com/articles/2021-test-shapes.html), there's two lines of thought: solitary unit tests and sociable unit tests. Within MetaMask there are multiple examples from tests and discussions highlighting the two types (for example you can check the discussion on [this PR](https://www.google.com/url?q=https://github.com/MetaMask/core/pull/3827%23discussion_r1469377179&sa=D&source=editors&ust=1716475694980436&usg=AOvVaw3oTcfVgfRuiQFSDChZPrFM))


Suggested change

Unit tests understanding is a bit more diffuse. As there is no consensus on how they should be written and what they should test. But generically, [as captured by Martin Fowler](https://martinfowler.com/articles/2021-test-shapes.html), there's two lines of thought: solitary unit tests and sociable unit tests. Within MetaMask there are multiple examples from tests and discussions highlighting the two types (for example you can check the discussion on [this PR](https://www.google.com/url?q=https://github.com/MetaMask/core/pull/3827%23discussion_r1469377179&sa=D&source=editors&ust=1716475694980436&usg=AOvVaw3oTcfVgfRuiQFSDChZPrFM))

Unit testing is a conceptually vague practice, as there is no consensus on how they should be written and what they should test. But in general, [as outlined by Martin Fowler](https://martinfowler.com/articles/2021-test-shapes.html), there are two approaches: solitary unit tests and sociable unit tests. At MetaMask, we employ both of these types of unit tests (see [discussion on the use cases of each type](https://github.com/MetaMask/core/pull/3827#discussion_r1469377179)).

Unit tests understanding is a bit more diffuse. As there is no consensus on how they should be written and what they should test.

This is a bold, sweeping statement, especially given the near-universal usage of unit testing (and TDD) in the industry. Could you maybe provide more context on what you mean by this?

Also, perhaps we could use a less ambiguous word here than "diffuse"?

there's two lines of thought: solitary unit tests and sociable unit tests

Should we provide a summary of what these terms mean, how they are different, and why they are important? Are we expecting the reader of this doc to read through the linked blog article to find out?

Also, if these two terms are central to our understanding of unit testing, should they be discussed in the unit testing doc itself?

[this PR](https://www.google.com/url?q=https://github.com/MetaMask/core/pull/3827%23discussion_r1469377179&sa=D&source=editors&ust=1716475694980436&usg=AOvVaw3oTcfVgfRuiQFSDChZPrFM))

There are quite a few comments in the linked thread. Should we provide an indication of which parts of the discussion we're trying to emphasize, and how they are relevant to this paragraph? Should the reader of this doc be expected to read through the entire discussion thread and discern the implications themselves?

Also, is there a reason we're using a google redirect link here?

Ok this is a big comment, let me address each point:

This is a bold, sweeping statement, especially given the near-universal usage of unit testing (and TDD) in the industry. Could you maybe provide more context on what you mean by this?

Universal usage does not mean universal understanding on how to test a unit. And that section for the unit tests is there to recognise that there are different types of unit tests. Being the most common ones what Martin Fowler names solitary unit tests and sociable unit tests. And there are multiple examples of both in our codebase. The PR discussion highlighted, shows the two lines of though being discussed (i.e. where are the limits of a unit test, where and what should we mock).

Should we provide a summary of what these terms mean, how they are different, and why they are important? Are we expecting the reader of this doc to read through the linked blog article to find out?

Those could be added in the unit tests doc. I haven't added anything there, neither summarised the concept of sociable vs solitary unit tests as that was not the scope of this PR. The intended scope was to provide some generic guidelines on when to rely on each type of test (this is a follow up of adding the UI integration tests into the Extension).

Also, if these two terms are central to our understanding of unit testing, should they be discussed in the unit testing doc itself?

Yes. But maybe on a different PR where we expand our unit test documentation.

Should the reader of this doc be expected to read through the entire discussion thread and discern the implications themselves?

I honestly think that it would be good to read the entire thread, so that reader can get a good sense on how different developers where advocating for different patterns without realising that the limits of a unit test are quite different for each.

Also, is there a reason we're using a google redirect link here?

No.. Not sure how that turned into a google redirect link 🤦‍♂️ . Fixing it.

MajorLift · 2024-07-12T15:26:39Z

docs/test-guidelines.md

+- **Code fencing:** is very problematic for unit/integration testing. We should avoid it.
+- **jest manual mocks:** This types of mocks are automatically picked up by jest for all tests. We should be very careful when writing this types of mocks as they will be shared across all tests. And with integration tests, as we are aiming to mock as little as possible, we should avoid this at all costs.
+
+## Conclusion
+
+To sum up, understanding the distinction between e2e, integration and unit tests and applying them appropriately is crucial for maintaining MetaMask's code quality and reliability. Integration tests offer a broad view of user interactions and system integration for page-level components, while unit tests provide detailed insights into the functionality of individual components,and e2e tests validate the application's overall behavior, ensuring readiness for production.
+By following these guidelines, developers can ensure comprehensive test coverage, contributing to the robustness and user satisfaction of the application


fixes: typos, consistent style

Suggested change

- **Code fencing:** is very problematic for unit/integration testing. We should avoid it.

- **jest manual mocks:** This types of mocks are automatically picked up by jest for all tests. We should be very careful when writing this types of mocks as they will be shared across all tests. And with integration tests, as we are aiming to mock as little as possible, we should avoid this at all costs.

## Conclusion

To sum up, understanding the distinction between e2e, integration and unit tests and applying them appropriately is crucial for maintaining MetaMask's code quality and reliability. Integration tests offer a broad view of user interactions and system integration for page-level components, while unit tests provide detailed insights into the functionality of individual components,and e2e tests validate the application's overall behavior, ensuring readiness for production.

By following these guidelines, developers can ensure comprehensive test coverage, contributing to the robustness and user satisfaction of the application

- **Code Fencing:** This practice is very problematic for unit/integration testing. We should avoid it.

- **Jest Manual Mocks:** These mocks are automatically picked up by jest for all tests. We should be very careful when writing this types of mocks as they will be shared across all tests. And with integration tests, as we are aiming to mock as little as possible, we should avoid this at all costs.

## Conclusion

To sum up, understanding the distinction between e2e, integration and unit tests and applying them appropriately is crucial for maintaining MetaMask's code quality and reliability. Integration tests offer a broad view of user interactions and system integration for page-level components, while unit tests provide detailed insights into the functionality of individual components, and e2e tests validate the application's overall behavior, ensuring readiness for production.

By following these guidelines, developers can ensure comprehensive test coverage, while contributing to the robustness and user satisfaction of the application.

seaona · 2024-07-15T07:31:55Z

docs/test-guidelines.md

@@ -0,0 +1,57 @@
+# Test guidelines


About the file location, file name and content organization, we currently have these 2 files:

docs/tests/jest.md

docs/testing.md

I think if we add a third file called docs/test-guidelines.md might be a bit confusing over what we would expect here. Could we maybe rethink the file name or the content organization itself?

If we decide to have this information on a separate file, as in this PR, maybe we could link it on the testing.md file, as well as re-name it, so it's a bit more explicit what this is about?

humm not sure I follow. This is on the contributors doc repo.. and the screenshot you posted is from extension repo. Are you suggesting adding a link to this from the Extension repo docs? Or adding this doc to the Extension repo docs instead of adding it here?

I like how this document broadly covers the purpose of testing and how we use it at MetaMask.

Are you familiar with the Diataxis system? This is a way of arranging information that I found while during research prior to setting up this repo. Basically it suggests that there are different kinds of documentation that serve different purposes. When I wrote the unit testing guidelines I provided some background information for each guideline but I tried to keep the overall document squarely in the "reference" category. The idea is that people could point people to a particular guideline in a PR or on Slack without needing to read the entire document.

This document seems more of an "explanation" kind of document. That's great, I think we've reached the point where we need some things like this. I just wanted to point out that perhaps it's worth thinking about that distinction, and I wonder if we ought to gear this document toward that. I notice that the name of this file is "Test guidelines" and how this file lists some guidelines for each testing category but also provides a summary. I wonder if we want to extract all guidelines in separate documents and just use this file to provide a general overview for each kind of testing category?

In addition, perhaps we have a testing/ directory and we rename this to testing/overview.md or something like that, and then we can relocate the existing unit-testing.md to testing/unit-testing-guidelines.md or something?

There are many ways to skin this cat, so to speak, so I'm curious to know what you think.

Thanks for this @mcmire. I was not familiar with the Diataxis system. It seems to be a great guide to differentiate types of documentation. I looked into the "explanation" details and tried to update the current overview to be more inline with it.
Also, as you mentioned, I took the opportunity to re-organise the test documentation. Please have a look and let me know your thoughts.

cryptotavares · 2024-07-15T16:47:11Z

I have a few questions about the overall scope of the doc:

Is this doc intended to only cover UI testing? The references to "page-level components" make it difficult to apply this doc to any testing that doesn't involve the presentation layer. For example, is the section on integration testing relevant to how we would test interactions between multiple MetaMask controllers?

Also, I'm a little confused as to what this doc is trying to achieve. If we are providing additional insights about each type of tests, shouldn't those insights be added to the testing docs themselves? Are we essentially providing a consolidated table of contents for our testing docs? Or are we trying to convey insights that encompass multiple types of tests and therefore don't belong in any single testing doc?

Oh yes, this is just UI integration tests. Not sure if we have integration tests for controllers (I don't think that we do right?). I'll update all references of Integration tests to UI Integration tests.

The goal of this doc was to provide an overview of the different types of tests that we support and write some generic guidelines that help the contributor to known when to implement each type of test. It is especially important as we have introduced UI integration tests into MetaMask Extension (and will most likely soon implement those on MetaMask Mobile). But it is not intended to detail best practices on how to write the test itself.
Maybe I should consolidate the docs structure for tests in this PR? Having a test directory, with a main readme (which could be this one) and then docs for all the test types? 🤔

@MajorLift really appreciate your comments. Let me know your thoughts.

mcmire

Had a few more suggestions. I'm out on leave soon, so I won't be able to give you a final approval, but this looks great overall!

mcmire · 2024-07-26T20:25:08Z

docs/testing/overview.md

+
+## What are unit tests?
+
+Unit testing is a conceptually vague practice, as there is no consensus on how they should be written and what they should test. But in general, [as outlined by Martin Fowler](https://martinfowler.com/articles/2021-test-shapes.html), there two common approaches: solitary unit tests and sociable unit tests. At MetaMask, we employ both of these types of unit tests (see [discussion on the use cases of each type](https://github.com/MetaMask/core/pull/3827#discussion_r1469377179))


Nit: typo (missing "are"):

Suggested change

Unit testing is a conceptually vague practice, as there is no consensus on how they should be written and what they should test. But in general, [as outlined by Martin Fowler](https://martinfowler.com/articles/2021-test-shapes.html), there two common approaches: solitary unit tests and sociable unit tests. At MetaMask, we employ both of these types of unit tests (see [discussion on the use cases of each type](https://github.com/MetaMask/core/pull/3827#discussion_r1469377179))

Unit testing is a conceptually vague practice, as there is no consensus on how they should be written and what they should test. But in general, [as outlined by Martin Fowler](https://martinfowler.com/articles/2021-test-shapes.html), there are two common approaches: solitary unit tests and sociable unit tests. At MetaMask, we employ both of these types of unit tests (see [discussion on the use cases of each type](https://github.com/MetaMask/core/pull/3827#discussion_r1469377179))

mcmire · 2024-07-26T20:25:51Z

docs/testing/overview.md

+### When to write unit tests?
+
+All components or functions should be validated through unit tests. These tests focus on the component's or function's internal logic and functionality.
+If UI Integration tests are available, those are preferred for page-level components. However, developers may choose to write sociable unit tests for specific scenarios where UI integration tests might not be necessary or practical.


Nit: lowercasing "i" to match other usage:

Suggested change

If UI Integration tests are available, those are preferred for page-level components. However, developers may choose to write sociable unit tests for specific scenarios where UI integration tests might not be necessary or practical.

If UI integration tests are available, those are preferred for page-level components. However, developers may choose to write sociable unit tests for specific scenarios where UI integration tests might not be necessary or practical.

mcmire · 2024-07-26T20:26:35Z

docs/testing/overview.md

+
+## What are end-to-end tests?
+
+End-to-end tests simulate real user scenarios from start to finish, testing the application as a whole. In the Extension, end-to-end tests are tests that strive to test a real extension app (including background, contentscript and ui processes) in a controlled environment from a user perspective.


Nit: Adding backticks around components of the Extension:

Suggested change

End-to-end tests simulate real user scenarios from start to finish, testing the application as a whole. In the Extension, end-to-end tests are tests that strive to test a real extension app (including background, contentscript and ui processes) in a controlled environment from a user perspective.

End-to-end tests simulate real user scenarios from start to finish, testing the application as a whole. In the Extension, end-to-end tests are tests that strive to test a real extension app (including `background`, `contentscript` and `ui` processes) in a controlled environment from a user perspective.

mcmire · 2024-07-26T20:28:04Z

docs/testing/overview.md

+
+### When to write end-to-end tests?
+
+Testing application's integration with external systems or services through critical user journeys. This includes any interactions between the UI, background process, dapp, blockchain, etc. End-to-end tests should closely mimic real user actions and flows.


Maybe this would read more smoothly?

Suggested change

Testing application's integration with external systems or services through critical user journeys. This includes any interactions between the UI, background process, dapp, blockchain, etc. End-to-end tests should closely mimic real user actions and flows.

End-to-end tests exercise an application's integration with external systems or services through critical user journeys. This includes any interactions between the UI, background process, dapp, blockchain, etc. End-to-end tests should closely mimic real user actions and flows.

mcmire · 2024-07-26T20:28:20Z

docs/testing/overview.md

+
+## Generic testing considerations
+
+- **Code fencing:** Code fencing is available in MetaMask Extension and Mobile codebases. Though usefull for its purposes, this practice is very problematic for unit/UI integration testing. We should avoid it.


Nit: typo:

Suggested change

- **Code fencing:** Code fencing is available in MetaMask Extension and Mobile codebases. Though usefull for its purposes, this practice is very problematic for unit/UI integration testing. We should avoid it.

- **Code fencing:** Code fencing is available in MetaMask Extension and Mobile codebases. Though useful for its purposes, this practice is very problematic for unit/UI integration testing. We should avoid it.

mcmire · 2024-07-26T20:29:19Z

docs/testing/ui-integration-testing.md

+
+UI integration tests should focus on user journeys and interactions within the application. Design tests to mimic actual user actions and flows. This helps in identifying issues that could affect the user experience.
+
+### Full App Context


What are your thoughts on making each header a command?

Suggested change

### Full App Context

### Provide Full App Context

mcmire · 2024-07-26T20:30:07Z

docs/testing/ui-integration-testing.md

+
+UI integration tests should not be written for components other than page-level components. Other components should be tested using unit tests.
+
+## Test Location


Similar to above comment:

Suggested change

## Test Location

## Keep Tests Separate from Implementation

mcmire · 2024-07-26T20:30:21Z

docs/testing/ui-integration-testing.md

+
+Refrain from using snapshot testing in UI integration tests. They tend to be less meaningful in the context of full app testing and can lead to brittle tests.
+
+## No Mocking UI Components


Similar comment as above:

Suggested change

## No Mocking UI Components

## Don't Mock UI Components

mcmire · 2024-07-26T20:34:02Z

docs/testing/unit-testing.md

@@ -1070,3 +1070,5 @@ Jest incorporates most of the features of Sinon with a slimmer API:
 - `jest.fn()` can be used in place of `sinon.stub()`.
 - `jest.spyOn(object, method)` can be used in place of `sinon.spy(object, method)` or `sinon.stub(object, method)` (with the caveat that the method being spied upon will still be called by default).
 - `jest.useFakeTimers()` can be used in place of `sinon.useFakeTimers()` (though note that Jest's "clock" object had fewer features than Sinon's prior to Jest v29.5).
+
+**Avoid jest manual mocks:** According to jest `Manual mocks are defined by writing a module in a __mocks__/ subdirectory immediately`. These types of mocks are automatically picked up by jest for all tests. We should be very careful when writing this types of mocks as they will be shared across all tests. And with UI integration tests, as we are aiming to mock as little as possible, we should avoid this at all costs.


Hmm, I see why you included this here, but maybe we should rename this section from "Use Jest's mock functions instead of Sinon" to "Use Jest's API instead of Sinon's"?

Additionally, to make this guideline stand out and make it linkable on its own, should we move it to its own header? Finally, since this is the document on unit testing, do we need to include anything around integration testing here?

Suggested change

**Avoid jest manual mocks:** According to jest `Manual mocks are defined by writing a module in a __mocks__/ subdirectory immediately`. These types of mocks are automatically picked up by jest for all tests. We should be very careful when writing this types of mocks as they will be shared across all tests. And with UI integration tests, as we are aiming to mock as little as possible, we should avoid this at all costs.

## Avoid manual mocks

According to Jest's documentation, "Manual mocks are defined by writing a module in a `__mocks__/` subdirectory immediately". These types of mocks are automatically picked up by Jest for all tests. We should be very careful when writing these types of mocks as they will be shared across all tests.

mcmire · 2024-07-26T20:34:11Z

README.md

@@ -12,5 +12,7 @@ This is a living repository — nothing is set in stone! If you're member of Met
 - [Secure Development Lifecycle Policy](./docs/sdlc.md)
 - [Secure Coding Guidelines](./docs/secure-coding-guidelines.md)
 - [TypeScript Guidelines](./docs/typescript.md)
- [Unit Testing Guidelines](./docs/unit-testing.md)
- [E2E Testing Guidelines](./docs/e2e-testing.md)
+- [Testing Guidelines](./docs/testing/overview.md)


This is great! I like this arrangement.

Add generic guidelines about when to rely on e2e, integration or unit tests.

Created a test directory with an overview doc and docs with specific guidelines for each test type

chloeYue

LGTM

digiwand · 2024-09-17T03:06:50Z

docs/testing/unit-testing.md

@@ -1071,6 +1071,10 @@ Jest incorporates most of the features of Sinon with a slimmer API:
 - `jest.spyOn(object, method)` can be used in place of `sinon.spy(object, method)` or `sinon.stub(object, method)` (with the caveat that the method being spied upon will still be called by default).
 - `jest.useFakeTimers()` can be used in place of `sinon.useFakeTimers()` (though note that Jest's "clock" object had fewer features than Sinon's prior to Jest v29.5).

+## Avoid general manual mocks:
+
+According to Jest's documentation `Manual mocks are defined by writing a module in a __mocks__/ subdirectory immediately`. These types of mocks are automatically picked up by Jest for all tests. We should be very careful when writing this types of mocks as they will be shared across all tests (including UI integration tests).


It could be helpful to extend this suggestion to ui-integration-testing.md, possibly under "## Don't Mock UI Components". We could also append "(including unit tests)." to the end

digiwand · 2024-09-17T03:08:50Z

docs/testing/overview.md

+
+## What are unit tests?
+
+Unit testing is a conceptually vague practice, as there is no consensus on how they should be written and what they should test. But in general, [as outlined by Martin Fowler](https://martinfowler.com/articles/2021-test-shapes.html), there are two common approaches: solitary unit tests and sociable unit tests. At MetaMask, we employ both of these types of unit tests (see [discussion on the use cases of each type](https://github.com/MetaMask/core/pull/3827#discussion_r1469377179))


Suggested change

Unit testing is a conceptually vague practice, as there is no consensus on how they should be written and what they should test. But in general, [as outlined by Martin Fowler](https://martinfowler.com/articles/2021-test-shapes.html), there are two common approaches: solitary unit tests and sociable unit tests. At MetaMask, we employ both of these types of unit tests (see [discussion on the use cases of each type](https://github.com/MetaMask/core/pull/3827#discussion_r1469377179))

Unit testing is a conceptually vague practice, as there is no consensus on how they should be written and what they should test. But in general, [as outlined by Martin Fowler](https://martinfowler.com/articles/2021-test-shapes.html), there are two common approaches: solitary unit tests and sociable unit tests. At MetaMask, we employ both of these types of unit tests (see [discussion on the use cases of each type](https://github.com/MetaMask/core/pull/3827#discussion_r1469377179)).

pedronfigueiredo · 2024-09-17T09:18:13Z

docs/testing/ui-integration-testing.md

+
+## Don't Mock UI Components
+
+Keep mocking to minimum. Ideally only the background connection (MetaMask Extension), or any network request (fired from the UI) should be mocked.


It could be helpful to include a note about not mocking hooks, leveraging instead the natural boundary between UI and background as a place to mock.

MajorLift

I can see that the requests from Elliot and myself have been reflected. Thanks for your patience engaging with our comments. LGTM!

cryptodev-2s

LGTM!

cryptotavares requested a review from a team as a code owner July 11, 2024 12:00

cryptotavares mentioned this pull request Jul 11, 2024

test: Add UI integration tests MetaMask/metamask-extension#24428

Merged

7 tasks

cryptotavares force-pushed the cryptotavares/unit-integration-e2e-test-general-guidelines branch from dcded45 to cabbc34 Compare July 11, 2024 13:50

MajorLift reviewed Jul 12, 2024

View reviewed changes

MajorLift requested a review from a team July 12, 2024 16:01

MajorLift added team-wallet-framework and removed team-wallet-framework labels Jul 12, 2024

seaona reviewed Jul 15, 2024

View reviewed changes

cryptotavares force-pushed the cryptotavares/unit-integration-e2e-test-general-guidelines branch 3 times, most recently from f8a66e8 to 854e10d Compare July 19, 2024 10:56

mcmire reviewed Jul 26, 2024

View reviewed changes

cryptotavares force-pushed the cryptotavares/unit-integration-e2e-test-general-guidelines branch from 854e10d to 8d647b4 Compare September 4, 2024 15:51

cryptotavares added 3 commits September 10, 2024 23:04

chore: test guidelines

14a19e4

Add generic guidelines about when to rely on e2e, integration or unit tests.

chore: reorganise test docs

eac213c

Created a test directory with an overview doc and docs with specific guidelines for each test type

chore: minor fixes and improvements

2ccc5b3

cryptotavares force-pushed the cryptotavares/unit-integration-e2e-test-general-guidelines branch from 8d647b4 to 2ccc5b3 Compare September 10, 2024 22:06

chloeYue previously approved these changes Sep 12, 2024

View reviewed changes

digiwand reviewed Sep 17, 2024

View reviewed changes

pedronfigueiredo reviewed Sep 17, 2024

View reviewed changes

zone-live previously approved these changes Sep 18, 2024

View reviewed changes

NeMeC777 approved these changes Sep 26, 2024

View reviewed changes

fix: extend mocking rules for ui integrationt tests

216c33e

cryptotavares dismissed stale reviews from zone-live and chloeYue via 216c33e September 30, 2024 12:56

MajorLift approved these changes Sep 30, 2024

View reviewed changes

cryptodev-2s approved these changes Sep 30, 2024

View reviewed changes

cryptotavares merged commit dd3c581 into main Oct 1, 2024
6 checks passed

cryptotavares deleted the cryptotavares/unit-integration-e2e-test-general-guidelines branch October 1, 2024 17:48

Gudahtt mentioned this pull request Nov 7, 2024

Document our philosophy on Testing #5

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: add comprehensive testing guidelines #99

chore: add comprehensive testing guidelines #99

cryptotavares commented Jul 11, 2024

hjetpoluru commented Jul 11, 2024

MajorLift left a comment •

edited

Loading

MajorLift Jul 12, 2024 •

edited

Loading

cryptotavares Jul 15, 2024

MajorLift Jul 12, 2024 •

edited

Loading

seaona Jul 15, 2024

cryptotavares Jul 15, 2024

mcmire Jul 15, 2024 •

edited

Loading

cryptotavares Jul 19, 2024

cryptotavares commented Jul 15, 2024

mcmire left a comment

mcmire Jul 26, 2024

mcmire Jul 26, 2024

mcmire Jul 26, 2024

mcmire Jul 26, 2024

mcmire Jul 26, 2024

mcmire Jul 26, 2024

mcmire Jul 26, 2024

mcmire Jul 26, 2024

mcmire Jul 26, 2024

mcmire Jul 26, 2024

chloeYue left a comment

digiwand Sep 17, 2024

digiwand Sep 17, 2024

pedronfigueiredo Sep 17, 2024

MajorLift left a comment

cryptodev-2s left a comment


		## What are unit tests?

		Unit tests understanding is a bit more diffuse. As there is no consensus on how they should be written and what they should test. But generically, [as captured by Martin Fowler](https://martinfowler.com/articles/2021-test-shapes.html), there's two lines of thought: solitary unit tests and sociable unit tests. Within MetaMask there are multiple examples from tests and discussions highlighting the two types (for example you can check the discussion on [this PR](https://www.google.com/url?q=https://github.com/MetaMask/core/pull/3827%23discussion_r1469377179&sa=D&source=editors&ust=1716475694980436&usg=AOvVaw3oTcfVgfRuiQFSDChZPrFM))


		## What are unit tests?

		Unit testing is a conceptually vague practice, as there is no consensus on how they should be written and what they should test. But in general, [as outlined by Martin Fowler](https://martinfowler.com/articles/2021-test-shapes.html), there two common approaches: solitary unit tests and sociable unit tests. At MetaMask, we employ both of these types of unit tests (see [discussion on the use cases of each type](https://github.com/MetaMask/core/pull/3827#discussion_r1469377179))

	If UI Integration tests are available, those are preferred for page-level components. However, developers may choose to write sociable unit tests for specific scenarios where UI integration tests might not be necessary or practical.
	If UI integration tests are available, those are preferred for page-level components. However, developers may choose to write sociable unit tests for specific scenarios where UI integration tests might not be necessary or practical.


		## What are end-to-end tests?

		End-to-end tests simulate real user scenarios from start to finish, testing the application as a whole. In the Extension, end-to-end tests are tests that strive to test a real extension app (including background, contentscript and ui processes) in a controlled environment from a user perspective.


		### When to write end-to-end tests?

		Testing application's integration with external systems or services through critical user journeys. This includes any interactions between the UI, background process, dapp, blockchain, etc. End-to-end tests should closely mimic real user actions and flows.

	Testing application's integration with external systems or services through critical user journeys. This includes any interactions between the UI, background process, dapp, blockchain, etc. End-to-end tests should closely mimic real user actions and flows.
	End-to-end tests exercise an application's integration with external systems or services through critical user journeys. This includes any interactions between the UI, background process, dapp, blockchain, etc. End-to-end tests should closely mimic real user actions and flows.


		## Generic testing considerations

		- Code fencing: Code fencing is available in MetaMask Extension and Mobile codebases. Though usefull for its purposes, this practice is very problematic for unit/UI integration testing. We should avoid it.


		UI integration tests should focus on user journeys and interactions within the application. Design tests to mimic actual user actions and flows. This helps in identifying issues that could affect the user experience.

		### Full App Context


		UI integration tests should not be written for components other than page-level components. Other components should be tested using unit tests.

		## Test Location


		Refrain from using snapshot testing in UI integration tests. They tend to be less meaningful in the context of full app testing and can lead to brittle tests.

		## No Mocking UI Components

-**Avoid jest manual mocks:** According to jest `Manual mocks are defined by writing a module in a __mocks__/ subdirectory immediately`. These types of mocks are automatically picked up by jest for all tests. We should be very careful when writing this types of mocks as they will be shared across all tests. And with UI integration tests, as we are aiming to mock as little as possible, we should avoid this at all costs.
+## Avoid manual mocks
+According to Jest's documentation, "Manual mocks are defined by writing a module in a `__mocks__/` subdirectory immediately". These types of mocks are automatically picked up by Jest for all tests. We should be very careful when writing these types of mocks as they will be shared across all tests.


		## Don't Mock UI Components

		Keep mocking to minimum. Ideally only the background connection (MetaMask Extension), or any network request (fired from the UI) should be mocked.

chore: add comprehensive testing guidelines #99

chore: add comprehensive testing guidelines #99

Conversation

cryptotavares commented Jul 11, 2024

Description

Changes

hjetpoluru commented Jul 11, 2024

MajorLift left a comment • edited Loading

Choose a reason for hiding this comment

MajorLift Jul 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MajorLift Jul 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcmire Jul 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cryptotavares commented Jul 15, 2024

mcmire left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chloeYue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MajorLift left a comment

Choose a reason for hiding this comment

cryptodev-2s left a comment

Choose a reason for hiding this comment

MajorLift left a comment •

edited

Loading

MajorLift Jul 12, 2024 •

edited

Loading

MajorLift Jul 12, 2024 •

edited

Loading

mcmire Jul 15, 2024 •

edited

Loading