From 69b5552b852de8e6c019ab305eb970b31e592c14 Mon Sep 17 00:00:00 2001 From: tommasini Date: Tue, 18 Jun 2024 14:16:23 +0100 Subject: [PATCH 01/15] migrations guidelines --- docs/migrations-guidelines.md | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 docs/migrations-guidelines.md diff --git a/docs/migrations-guidelines.md b/docs/migrations-guidelines.md new file mode 100644 index 0000000..f970586 --- /dev/null +++ b/docs/migrations-guidelines.md @@ -0,0 +1,35 @@ +# Migration and Testing Guidelines + +## Overview + +This document outlines best practices and guidelines for writing migration scripts and their corresponding test cases. The focus is on ensuring data integrity, handling errors gracefully, and maintaining consistency across mobile app versions. + +## Migration Guidelines + +1. **Pre-Validation Checks**: Before proceeding with the migration logic, ensure the state to be migrated is valid. Use utility functions like `ensureValidState` to verify the version and integrity of the state. + +2. **Error Handling**: Use `captureException` from `@sentry/react-native` to log errors, which is crucial for diagnosing issues post-migration. Ensure that error messages are descriptive, including the migration number and a clear description of the issue. If an exception is detected, indicating potential data corruption, halt the migration process and return the state as is. This approach prevents further manipulation of potentially corrupted data, emphasizing the need for corrective action before proceeding. + +3. **State Integrity Checks**: Perform thorough checks on the state's structure and types. Use functions like `isObject` and `hasProperty` to validate the state and its nested properties. Log errors and halt the migration for any inconsistencies to prevent data corruption. + +4. **Avoid Type Casting**: Refrain from using type casting (e.g., `as` keyword in TypeScript) to manipulate the state or its properties. Type casting can mask underlying data structure issues, making the code less resilient to changes in dependencies or data structures. Instead, use type guards and runtime checks to ensure data integrity and compatibility. + +5. **Return State**: Always return the state at the end of the migration function, whether it was modified or not. This ensures that the migration process completes and the state continues through any subsequent migrations. + +## Testing Guidelines + +1. **Mocking**: Use `jest.mock` to mock external dependencies, such as `@sentry/react-native`. This isolates the migration test and prevents external side effects. + +2. **Initial State Setup**: Create an initial state that reflects possible real-world scenarios, including edge cases. Use utilities like `merge` from `lodash` to combine base states with specific test case modifications. + +3. **Invalid State Scenarios**: Test how the migration handles invalid states. This includes null values, incorrect types, and missing properties. Ensure that the migration logs the appropriate errors without modifying the state. + +4. **Data Synchronization Tests**: For migrations that synchronize data between controllers, write tests that verify the correct synchronization of data. Check for both the presence and accuracy of the synchronized data. + +5. **Error Assertions**: Verify that errors are logged correctly for invalid states or unexpected conditions. Use `expect` to assert that `captureException` was called with the expected error messages. + +6. **Ensure State Immutability**: Always use deep cloning (e.g., `cloneDeep` from `lodash`) on the old state before passing it to the migration function in tests. This practice ensures that the original state object is not mutated during the migration process, preserving the integrity of your test data across different test cases. Mutating the state directly can lead to hard-to-track bugs and false positives or negatives in your tests because subsequent tests might not start with the original state as intended. Deep cloning guarantees that each test case operates on an exact, untouched copy of the state, ensuring test reliability and accuracy. + +## Conclusion + +Following these guidelines will help ensure that migrations are robust, error-resistant, and maintain data integrity. Testing migrations thoroughly is crucial for identifying potential issues before they affect users. Always aim for clarity and thoroughness in both migration scripts and tests. \ No newline at end of file From e47dd51aedd00edf1aba9be95f6db6e90c463b2b Mon Sep 17 00:00:00 2001 From: tommasini Date: Tue, 18 Jun 2024 15:38:24 +0100 Subject: [PATCH 02/15] fix lint issues --- docs/migrations-guidelines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/migrations-guidelines.md b/docs/migrations-guidelines.md index f970586..29c1270 100644 --- a/docs/migrations-guidelines.md +++ b/docs/migrations-guidelines.md @@ -32,4 +32,4 @@ This document outlines best practices and guidelines for writing migration scrip ## Conclusion -Following these guidelines will help ensure that migrations are robust, error-resistant, and maintain data integrity. Testing migrations thoroughly is crucial for identifying potential issues before they affect users. Always aim for clarity and thoroughness in both migration scripts and tests. \ No newline at end of file +Following these guidelines will help ensure that migrations are robust, error-resistant, and maintain data integrity. Testing migrations thoroughly is crucial for identifying potential issues before they affect users. Always aim for clarity and thoroughness in both migration scripts and tests. From 6e5f8b13413ff6b4c8bd68ff42c70c5de06dc4d8 Mon Sep 17 00:00:00 2001 From: Nico MASSART Date: Tue, 18 Jun 2024 18:17:42 +0200 Subject: [PATCH 03/15] Update docs/migrations-guidelines.md --- docs/migrations-guidelines.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/migrations-guidelines.md b/docs/migrations-guidelines.md index 29c1270..30ca4a5 100644 --- a/docs/migrations-guidelines.md +++ b/docs/migrations-guidelines.md @@ -2,7 +2,8 @@ ## Overview -This document outlines best practices and guidelines for writing migration scripts and their corresponding test cases. The focus is on ensuring data integrity, handling errors gracefully, and maintaining consistency across mobile app versions. +This document outlines best practices and guidelines for writing migration scripts and their corresponding test cases. +The focus is on ensuring data integrity, handling errors gracefully, and maintaining consistency across mobile app versions. ## Migration Guidelines From 89489629e68196c8c66f43994ebcaa9664be53be Mon Sep 17 00:00:00 2001 From: Nico MASSART Date: Tue, 18 Jun 2024 18:18:24 +0200 Subject: [PATCH 04/15] Apply suggestions from code review --- docs/migrations-guidelines.md | 60 ++++++++++++++++++++++++++++------- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/docs/migrations-guidelines.md b/docs/migrations-guidelines.md index 30ca4a5..3e5ee5f 100644 --- a/docs/migrations-guidelines.md +++ b/docs/migrations-guidelines.md @@ -7,30 +7,66 @@ The focus is on ensuring data integrity, handling errors gracefully, and maintai ## Migration Guidelines -1. **Pre-Validation Checks**: Before proceeding with the migration logic, ensure the state to be migrated is valid. Use utility functions like `ensureValidState` to verify the version and integrity of the state. +1. **Pre-Validation Checks**: + - Ensure the state to be migrated is valid before running the migration logic, + - Verify the version and integrity of the state with utility functions like `ensureValidState`. -2. **Error Handling**: Use `captureException` from `@sentry/react-native` to log errors, which is crucial for diagnosing issues post-migration. Ensure that error messages are descriptive, including the migration number and a clear description of the issue. If an exception is detected, indicating potential data corruption, halt the migration process and return the state as is. This approach prevents further manipulation of potentially corrupted data, emphasizing the need for corrective action before proceeding. +2. **Error Handling**: -3. **State Integrity Checks**: Perform thorough checks on the state's structure and types. Use functions like `isObject` and `hasProperty` to validate the state and its nested properties. Log errors and halt the migration for any inconsistencies to prevent data corruption. +The following prevents manipulation of potentially corrupted data, enforcing corrective action before proceeding. -4. **Avoid Type Casting**: Refrain from using type casting (e.g., `as` keyword in TypeScript) to manipulate the state or its properties. Type casting can mask underlying data structure issues, making the code less resilient to changes in dependencies or data structures. Instead, use type guards and runtime checks to ensure data integrity and compatibility. + - Log erros with `captureException` from `@sentry/react-native`, which is crucial for diagnosing issues post-migration, + - Ensure that error messages are descriptive: include the migration number and a clear description of the issue, + - If an exception is detected, indicating potential data corruption, halt the migration process and return the intial state, -5. **Return State**: Always return the state at the end of the migration function, whether it was modified or not. This ensures that the migration process completes and the state continues through any subsequent migrations. +3. **State Integrity Checks**: + - Checks the state's structure and types carefully, + - Validate the state and its nested properties using functions like `isObject` and `hasProperty`, + - Prevent data corruption by halting the migration on any inconsistencies and logging errors. + +4. **Avoid Type Casting**: + - Do not use type casting (for example, `as` keyword in TypeScript) to manipulate the state or its properties, + - Type casting can mask underlying data structure issues and make the code less resilient in case of change in dependencies or data structures, + - Instead, use type guards and runtime checks to ensure data integrity and compatibility. + +5. **Return State**: + - Always return the state at the end of the migration function, whether it was modified or not, + - Returning the state ensures that the migration process completes and the state is passed to the next migrations. ## Testing Guidelines -1. **Mocking**: Use `jest.mock` to mock external dependencies, such as `@sentry/react-native`. This isolates the migration test and prevents external side effects. +1. **Mocking**: + - Mock dependencies with `jest.mock` (for example `@sentry/react-native`), + - Mocking isolates the migration under test and prevents side effects. -2. **Initial State Setup**: Create an initial state that reflects possible real-world scenarios, including edge cases. Use utilities like `merge` from `lodash` to combine base states with specific test case modifications. +2. **Initial State Setup**: + - Create an initial state that reflects possible real-world scenarios, including edge cases, + - if needed, create multiple initial states and use them each in a test for this specific case, + - Use utilities like `merge` from `lodash` to combine base states with specific test case modifications. -3. **Invalid State Scenarios**: Test how the migration handles invalid states. This includes null values, incorrect types, and missing properties. Ensure that the migration logs the appropriate errors without modifying the state. +3. **Invalid State Scenarios**: + - Test how the migration handles invalid states, including null values, incorrect types, and missing propertie, + - Ensure that the migration logs the appropriate errors without modifying and corrupting the state. -4. **Data Synchronization Tests**: For migrations that synchronize data between controllers, write tests that verify the correct synchronization of data. Check for both the presence and accuracy of the synchronized data. +4. **Data Synchronization Tests**: + - Write tests that verify the correct synchronization of data for migrations that synchronize data between controllers, + - Check the presence and accuracy of the synchronized data. -5. **Error Assertions**: Verify that errors are logged correctly for invalid states or unexpected conditions. Use `expect` to assert that `captureException` was called with the expected error messages. +5. **Error Assertions**: + - Verify that errors are logged correctly for invalid states or unexpected conditions, + - Use `expect` to assert that `captureException` was called with the expected error messages. -6. **Ensure State Immutability**: Always use deep cloning (e.g., `cloneDeep` from `lodash`) on the old state before passing it to the migration function in tests. This practice ensures that the original state object is not mutated during the migration process, preserving the integrity of your test data across different test cases. Mutating the state directly can lead to hard-to-track bugs and false positives or negatives in your tests because subsequent tests might not start with the original state as intended. Deep cloning guarantees that each test case operates on an exact, untouched copy of the state, ensuring test reliability and accuracy. +6. **Ensure State Immutability**: + - Always use deep cloning on the old state before passing it to the migration function in tests. For example, use `cloneDeep` from `lodash`. + - Deep cloning preserves the integrity of your test data across different test cases, + - Ensures the original state object is not mutated during the migration process, + - guarantees that each test case runs on an correct, clean copy of the state. + - Never mutate the state directly as this can: + - lead to hard-to-track bugs and false positives or negatives test results, + - start subsequent tests with the original state as intended. ## Conclusion -Following these guidelines will help ensure that migrations are robust, error-resistant, and maintain data integrity. Testing migrations thoroughly is crucial for identifying potential issues before they affect users. Always aim for clarity and thoroughness in both migration scripts and tests. +You will improve migrations robustness by following these guidelines and enforce error-resistance, and maintain data integrity. +You will identify more potential issues with extended testing of migrations before they reach our users. +Always aim for clarity and thoroughness in both migration scripts and tests. From 79cbf02216566ef4be7940a720b43e09bf9d598d Mon Sep 17 00:00:00 2001 From: Nico MASSART Date: Tue, 18 Jun 2024 18:20:06 +0200 Subject: [PATCH 05/15] Update docs/migrations-guidelines.md --- docs/migrations-guidelines.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/migrations-guidelines.md b/docs/migrations-guidelines.md index 3e5ee5f..97670c4 100644 --- a/docs/migrations-guidelines.md +++ b/docs/migrations-guidelines.md @@ -14,7 +14,6 @@ The focus is on ensuring data integrity, handling errors gracefully, and maintai 2. **Error Handling**: The following prevents manipulation of potentially corrupted data, enforcing corrective action before proceeding. - - Log erros with `captureException` from `@sentry/react-native`, which is crucial for diagnosing issues post-migration, - Ensure that error messages are descriptive: include the migration number and a clear description of the issue, - If an exception is detected, indicating potential data corruption, halt the migration process and return the intial state, From ce8b2f80702f5cf7a1d57a87c69cb636491ea175 Mon Sep 17 00:00:00 2001 From: Nico MASSART Date: Tue, 18 Jun 2024 18:22:44 +0200 Subject: [PATCH 06/15] Update migrations-guidelines.md fix markdown --- docs/migrations-guidelines.md | 96 ++++++++++++++++++++--------------- 1 file changed, 55 insertions(+), 41 deletions(-) diff --git a/docs/migrations-guidelines.md b/docs/migrations-guidelines.md index 97670c4..3722ca1 100644 --- a/docs/migrations-guidelines.md +++ b/docs/migrations-guidelines.md @@ -3,69 +3,83 @@ ## Overview This document outlines best practices and guidelines for writing migration scripts and their corresponding test cases. + The focus is on ensuring data integrity, handling errors gracefully, and maintaining consistency across mobile app versions. ## Migration Guidelines -1. **Pre-Validation Checks**: - - Ensure the state to be migrated is valid before running the migration logic, - - Verify the version and integrity of the state with utility functions like `ensureValidState`. +1. **Pre-Validation Checks**: + +- Ensure the state to be migrated is valid before running the migration logic, +- Verify the version and integrity of the state with utility functions like `ensureValidState`. 2. **Error Handling**: The following prevents manipulation of potentially corrupted data, enforcing corrective action before proceeding. - - Log erros with `captureException` from `@sentry/react-native`, which is crucial for diagnosing issues post-migration, - - Ensure that error messages are descriptive: include the migration number and a clear description of the issue, - - If an exception is detected, indicating potential data corruption, halt the migration process and return the intial state, -3. **State Integrity Checks**: - - Checks the state's structure and types carefully, - - Validate the state and its nested properties using functions like `isObject` and `hasProperty`, - - Prevent data corruption by halting the migration on any inconsistencies and logging errors. +- Log erros with `captureException` from `@sentry/react-native`, which is crucial for diagnosing issues post-migration, +- Ensure that error messages are descriptive: include the migration number and a clear description of the issue, +- If an exception is detected, indicating potential data corruption, halt the migration process and return the intial state, + +3. **State Integrity Checks**: + +- Checks the state's structure and types carefully, +- Validate the state and its nested properties using functions like `isObject` and `hasProperty`, +- Prevent data corruption by halting the migration on any inconsistencies and logging errors. -4. **Avoid Type Casting**: - - Do not use type casting (for example, `as` keyword in TypeScript) to manipulate the state or its properties, - - Type casting can mask underlying data structure issues and make the code less resilient in case of change in dependencies or data structures, - - Instead, use type guards and runtime checks to ensure data integrity and compatibility. +5. **Avoid Type Casting**: -5. **Return State**: - - Always return the state at the end of the migration function, whether it was modified or not, - - Returning the state ensures that the migration process completes and the state is passed to the next migrations. +- Do not use type casting (for example, `as` keyword in TypeScript) to manipulate the state or its properties, +- Type casting can mask underlying data structure issues and make the code less resilient in case of change in dependencies or data structures, +- Instead, use type guards and runtime checks to ensure data integrity and compatibility. + +6. **Return State**: + +- Always return the state at the end of the migration function, whether it was modified or not, +- Returning the state ensures that the migration process completes and the state is passed to the next migrations. ## Testing Guidelines -1. **Mocking**: - - Mock dependencies with `jest.mock` (for example `@sentry/react-native`), - - Mocking isolates the migration under test and prevents side effects. +1. **Mocking**: + +- Mock dependencies with `jest.mock` (for example `@sentry/react-native`), +- Mocking isolates the migration under test and prevents side effects. -2. **Initial State Setup**: - - Create an initial state that reflects possible real-world scenarios, including edge cases, - - if needed, create multiple initial states and use them each in a test for this specific case, - - Use utilities like `merge` from `lodash` to combine base states with specific test case modifications. +2. **Initial State Setup**: -3. **Invalid State Scenarios**: - - Test how the migration handles invalid states, including null values, incorrect types, and missing propertie, - - Ensure that the migration logs the appropriate errors without modifying and corrupting the state. +- Create an initial state that reflects possible real-world scenarios, including edge cases, +- if needed, create multiple initial states and use them each in a test for this specific case, +- Use utilities like `merge` from `lodash` to combine base states with specific test case modifications. -4. **Data Synchronization Tests**: - - Write tests that verify the correct synchronization of data for migrations that synchronize data between controllers, - - Check the presence and accuracy of the synchronized data. +4. **Invalid State Scenarios**: -5. **Error Assertions**: - - Verify that errors are logged correctly for invalid states or unexpected conditions, - - Use `expect` to assert that `captureException` was called with the expected error messages. +- Test how the migration handles invalid states, including null values, incorrect types, and missing propertie, +- Ensure that the migration logs the appropriate errors without modifying and corrupting the state. -6. **Ensure State Immutability**: - - Always use deep cloning on the old state before passing it to the migration function in tests. For example, use `cloneDeep` from `lodash`. - - Deep cloning preserves the integrity of your test data across different test cases, - - Ensures the original state object is not mutated during the migration process, - - guarantees that each test case runs on an correct, clean copy of the state. - - Never mutate the state directly as this can: - - lead to hard-to-track bugs and false positives or negatives test results, - - start subsequent tests with the original state as intended. +5. **Data Synchronization Tests**: + +- Write tests that verify the correct synchronization of data for migrations that synchronize data between controllers, +- Check the presence and accuracy of the synchronized data. + +6. **Error Assertions**: + +- Verify that errors are logged correctly for invalid states or unexpected conditions, +- Use `expect` to assert that `captureException` was called with the expected error messages. + +8. **Ensure State Immutability**: + +- Always use deep cloning on the old state before passing it to the migration function in tests. For example, use `cloneDeep` from `lodash`. + - Deep cloning preserves the integrity of your test data across different test cases, + - Ensures the original state object is not mutated during the migration process, + - guarantees that each test case runs on an correct, clean copy of the state. +- Never mutate the state directly as this can: + - lead to hard-to-track bugs and false positives or negatives test results, + - start subsequent tests with the original state as intended. ## Conclusion You will improve migrations robustness by following these guidelines and enforce error-resistance, and maintain data integrity. + You will identify more potential issues with extended testing of migrations before they reach our users. + Always aim for clarity and thoroughness in both migration scripts and tests. From ab87df3544e709a938a214c610aa0ffc89cfaee6 Mon Sep 17 00:00:00 2001 From: tommasini Date: Tue, 18 Jun 2024 17:24:53 +0100 Subject: [PATCH 07/15] fix lint issues --- docs/migrations-guidelines.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/migrations-guidelines.md b/docs/migrations-guidelines.md index 3722ca1..0f19582 100644 --- a/docs/migrations-guidelines.md +++ b/docs/migrations-guidelines.md @@ -10,16 +10,16 @@ The focus is on ensuring data integrity, handling errors gracefully, and maintai 1. **Pre-Validation Checks**: -- Ensure the state to be migrated is valid before running the migration logic, +- Ensure the state to be migrated is valid before running the migration logic, - Verify the version and integrity of the state with utility functions like `ensureValidState`. -2. **Error Handling**: +2. **Error Handling**: The following prevents manipulation of potentially corrupted data, enforcing corrective action before proceeding. - Log erros with `captureException` from `@sentry/react-native`, which is crucial for diagnosing issues post-migration, - Ensure that error messages are descriptive: include the migration number and a clear description of the issue, -- If an exception is detected, indicating potential data corruption, halt the migration process and return the intial state, +- If an exception is detected, indicating potential data corruption, halt the migration process and return the intial state, 3. **State Integrity Checks**: From d0a979eb76cb785dd763135a74d6fff75f4c66bf Mon Sep 17 00:00:00 2001 From: tommasini Date: Wed, 19 Jun 2024 03:11:39 +0100 Subject: [PATCH 08/15] added example of migration, and also added to readme the migraitons guidelines md file --- README.md | 1 + docs/migrations-guidelines.md | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/README.md b/README.md index 3762e1d..8c230d2 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,7 @@ This is a living repository — nothing is set in stone! If you're member of Met - [Engineering Principles](./docs/engineering-principles.md) - [Guide to Pull Requests](./docs/pull-requests.md) - [JavaScript Guidelines](./docs/javascript.md) +- [Migrations Best Practices](./docs/migrations.md) - [Secure Development Lifecycle Policy](./docs/sdlc.md) - [Secure Coding Guidelines](./docs/secure-coding-guidelines.md) - [TypeScript Guidelines](./docs/typescript.md) diff --git a/docs/migrations-guidelines.md b/docs/migrations-guidelines.md index 0f19582..b034e2f 100644 --- a/docs/migrations-guidelines.md +++ b/docs/migrations-guidelines.md @@ -6,6 +6,10 @@ This document outlines best practices and guidelines for writing migration scrip The focus is on ensuring data integrity, handling errors gracefully, and maintaining consistency across mobile app versions. +We always look for improvement, if you see anything that could be improved, please open a PR against this guidelines. + +You can also check an example of a migration on MetaMask mobile app [here](https://github.com/MetaMask/metamask-mobile/blob/1855bd674e33bb0ece06fb6d8f09a4e5df46a108/app/store/migrations/044.ts#L1) + ## Migration Guidelines 1. **Pre-Validation Checks**: From f867d0492caf203f297ecaf3b1233034feb3d60c Mon Sep 17 00:00:00 2001 From: tommasini Date: Mon, 1 Jul 2024 12:13:18 +0100 Subject: [PATCH 09/15] migration docs updated with the reviews, now they are less verbose and only with important notes for the migration process in general --- README.md | 2 +- docs/migrations-guidelines.md | 55 ++++++++--------------------------- 2 files changed, 13 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 8c230d2..35f48c8 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ This is a living repository — nothing is set in stone! If you're member of Met - [Engineering Principles](./docs/engineering-principles.md) - [Guide to Pull Requests](./docs/pull-requests.md) - [JavaScript Guidelines](./docs/javascript.md) -- [Migrations Best Practices](./docs/migrations.md) +- [Migrations Best Practices](./docs/migrations-guidelines.md) - [Secure Development Lifecycle Policy](./docs/sdlc.md) - [Secure Coding Guidelines](./docs/secure-coding-guidelines.md) - [TypeScript Guidelines](./docs/typescript.md) diff --git a/docs/migrations-guidelines.md b/docs/migrations-guidelines.md index b034e2f..27ff182 100644 --- a/docs/migrations-guidelines.md +++ b/docs/migrations-guidelines.md @@ -4,7 +4,7 @@ This document outlines best practices and guidelines for writing migration scripts and their corresponding test cases. -The focus is on ensuring data integrity, handling errors gracefully, and maintaining consistency across mobile app versions. +The focus is on ensuring data integrity, handling errors gracefully, and maintaining consistency across our applications versions. We always look for improvement, if you see anything that could be improved, please open a PR against this guidelines. @@ -12,65 +12,42 @@ You can also check an example of a migration on MetaMask mobile app [here](https ## Migration Guidelines -1. **Pre-Validation Checks**: +1. **State Integrity Checks**: -- Ensure the state to be migrated is valid before running the migration logic, -- Verify the version and integrity of the state with utility functions like `ensureValidState`. +- State's on migrations are type `unknown`, it's crucial to validate state integrity before proceeding, we only migrate when structure and types meets our expectations, +- Validate the state and its nested properties using functions like `isObject` and `hasProperty` from `@metamask/utils`, +- Prevent data corruption by halting the migration on any inconsistencies and logging errors. 2. **Error Handling**: The following prevents manipulation of potentially corrupted data, enforcing corrective action before proceeding. -- Log erros with `captureException` from `@sentry/react-native`, which is crucial for diagnosing issues post-migration, +- Log errors with `captureException` from Sentry, which is crucial for diagnosing issues post-migration, - Ensure that error messages are descriptive: include the migration number and a clear description of the issue, - If an exception is detected, indicating potential data corruption, halt the migration process and return the intial state, -3. **State Integrity Checks**: - -- Checks the state's structure and types carefully, -- Validate the state and its nested properties using functions like `isObject` and `hasProperty`, -- Prevent data corruption by halting the migration on any inconsistencies and logging errors. - -5. **Avoid Type Casting**: - -- Do not use type casting (for example, `as` keyword in TypeScript) to manipulate the state or its properties, -- Type casting can mask underlying data structure issues and make the code less resilient in case of change in dependencies or data structures, -- Instead, use type guards and runtime checks to ensure data integrity and compatibility. - -6. **Return State**: +3. **Return State**: - Always return the state at the end of the migration function, whether it was modified or not, - Returning the state ensures that the migration process completes and the state is passed to the next migrations. ## Testing Guidelines -1. **Mocking**: - -- Mock dependencies with `jest.mock` (for example `@sentry/react-native`), -- Mocking isolates the migration under test and prevents side effects. - -2. **Initial State Setup**: +1. **Initial State Setup**: - Create an initial state that reflects possible real-world scenarios, including edge cases, - if needed, create multiple initial states and use them each in a test for this specific case, -- Use utilities like `merge` from `lodash` to combine base states with specific test case modifications. -4. **Invalid State Scenarios**: +2. **Invalid State Scenarios**: -- Test how the migration handles invalid states, including null values, incorrect types, and missing propertie, +- Test how the migration handles invalid states, including null values, incorrect types, and missing properties, - Ensure that the migration logs the appropriate errors without modifying and corrupting the state. -5. **Data Synchronization Tests**: - -- Write tests that verify the correct synchronization of data for migrations that synchronize data between controllers, -- Check the presence and accuracy of the synchronized data. - -6. **Error Assertions**: +3. **Error Assertions**: - Verify that errors are logged correctly for invalid states or unexpected conditions, -- Use `expect` to assert that `captureException` was called with the expected error messages. -8. **Ensure State Immutability**: +4. **Ensure State Immutability**: - Always use deep cloning on the old state before passing it to the migration function in tests. For example, use `cloneDeep` from `lodash`. - Deep cloning preserves the integrity of your test data across different test cases, @@ -79,11 +56,3 @@ The following prevents manipulation of potentially corrupted data, enforcing cor - Never mutate the state directly as this can: - lead to hard-to-track bugs and false positives or negatives test results, - start subsequent tests with the original state as intended. - -## Conclusion - -You will improve migrations robustness by following these guidelines and enforce error-resistance, and maintain data integrity. - -You will identify more potential issues with extended testing of migrations before they reach our users. - -Always aim for clarity and thoroughness in both migration scripts and tests. From 03a46c0bfc5c05809ed98796c575aef4360d4393 Mon Sep 17 00:00:00 2001 From: tommasini Date: Mon, 1 Jul 2024 12:20:40 +0100 Subject: [PATCH 10/15] remove description of error handling, that was most likely repeating what state integrety checks section was saying --- docs/migrations-guidelines.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/migrations-guidelines.md b/docs/migrations-guidelines.md index 27ff182..503b7a8 100644 --- a/docs/migrations-guidelines.md +++ b/docs/migrations-guidelines.md @@ -20,8 +20,6 @@ You can also check an example of a migration on MetaMask mobile app [here](https 2. **Error Handling**: -The following prevents manipulation of potentially corrupted data, enforcing corrective action before proceeding. - - Log errors with `captureException` from Sentry, which is crucial for diagnosing issues post-migration, - Ensure that error messages are descriptive: include the migration number and a clear description of the issue, - If an exception is detected, indicating potential data corruption, halt the migration process and return the intial state, From d0c82a9462ddac690bcdaae8001ae54b99f93594 Mon Sep 17 00:00:00 2001 From: tommasini Date: Mon, 1 Jul 2024 12:26:11 +0100 Subject: [PATCH 11/15] description for each section on migration guidelines --- docs/migrations-guidelines.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/migrations-guidelines.md b/docs/migrations-guidelines.md index 503b7a8..2efa353 100644 --- a/docs/migrations-guidelines.md +++ b/docs/migrations-guidelines.md @@ -14,18 +14,24 @@ You can also check an example of a migration on MetaMask mobile app [here](https 1. **State Integrity Checks**: +Validates the state's structure and types before migration, using specific functions to ensure data meets expectations. Halts migration if inconsistencies are detected, preventing data corruption. + - State's on migrations are type `unknown`, it's crucial to validate state integrity before proceeding, we only migrate when structure and types meets our expectations, - Validate the state and its nested properties using functions like `isObject` and `hasProperty` from `@metamask/utils`, - Prevent data corruption by halting the migration on any inconsistencies and logging errors. 2. **Error Handling**: +Logs detailed errors and halts the migration if potential data corruption is identified, ensuring issues are addressed before proceeding. + - Log errors with `captureException` from Sentry, which is crucial for diagnosing issues post-migration, - Ensure that error messages are descriptive: include the migration number and a clear description of the issue, - If an exception is detected, indicating potential data corruption, halt the migration process and return the intial state, 3. **Return State**: +Completes the migration by returning the state, modified or not, ensuring a seamless transition to subsequent migrations. + - Always return the state at the end of the migration function, whether it was modified or not, - Returning the state ensures that the migration process completes and the state is passed to the next migrations. From ed3961f960ce5b56e5775927007a6d925f577adb Mon Sep 17 00:00:00 2001 From: tommasini <46944231+tommasini@users.noreply.github.com> Date: Thu, 11 Jul 2024 01:53:03 +0100 Subject: [PATCH 12/15] Update docs/migrations-guidelines.md Co-authored-by: Cal Leung --- docs/migrations-guidelines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/migrations-guidelines.md b/docs/migrations-guidelines.md index 2efa353..94935ba 100644 --- a/docs/migrations-guidelines.md +++ b/docs/migrations-guidelines.md @@ -58,5 +58,5 @@ Completes the migration by returning the state, modified or not, ensuring a seam - Ensures the original state object is not mutated during the migration process, - guarantees that each test case runs on an correct, clean copy of the state. - Never mutate the state directly as this can: - - lead to hard-to-track bugs and false positives or negatives test results, + - lead to hard-to-track bugs and false positives or negatives test results. - start subsequent tests with the original state as intended. From ffdf7a21e66adc3f1c7950e5a6e73bf2b10f7db6 Mon Sep 17 00:00:00 2001 From: tommasini <46944231+tommasini@users.noreply.github.com> Date: Thu, 11 Jul 2024 01:53:11 +0100 Subject: [PATCH 13/15] Update docs/migrations-guidelines.md Co-authored-by: Cal Leung --- docs/migrations-guidelines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/migrations-guidelines.md b/docs/migrations-guidelines.md index 94935ba..44ce02c 100644 --- a/docs/migrations-guidelines.md +++ b/docs/migrations-guidelines.md @@ -55,7 +55,7 @@ Completes the migration by returning the state, modified or not, ensuring a seam - Always use deep cloning on the old state before passing it to the migration function in tests. For example, use `cloneDeep` from `lodash`. - Deep cloning preserves the integrity of your test data across different test cases, - - Ensures the original state object is not mutated during the migration process, + - Ensures the original state object is not mutated during the migration process. - guarantees that each test case runs on an correct, clean copy of the state. - Never mutate the state directly as this can: - lead to hard-to-track bugs and false positives or negatives test results. From 4095361530b1fdf7100fad598c6d0f03d322d8c5 Mon Sep 17 00:00:00 2001 From: tommasini <46944231+tommasini@users.noreply.github.com> Date: Thu, 11 Jul 2024 01:53:21 +0100 Subject: [PATCH 14/15] Update docs/migrations-guidelines.md Co-authored-by: Cal Leung --- docs/migrations-guidelines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/migrations-guidelines.md b/docs/migrations-guidelines.md index 44ce02c..063bf82 100644 --- a/docs/migrations-guidelines.md +++ b/docs/migrations-guidelines.md @@ -54,7 +54,7 @@ Completes the migration by returning the state, modified or not, ensuring a seam 4. **Ensure State Immutability**: - Always use deep cloning on the old state before passing it to the migration function in tests. For example, use `cloneDeep` from `lodash`. - - Deep cloning preserves the integrity of your test data across different test cases, + - Deep cloning preserves the integrity of your test data across different test cases. - Ensures the original state object is not mutated during the migration process. - guarantees that each test case runs on an correct, clean copy of the state. - Never mutate the state directly as this can: From c2f5b002a8637e76dfb2c89d8c9c5760547de5e0 Mon Sep 17 00:00:00 2001 From: tommasini Date: Tue, 23 Jul 2024 10:03:35 +0100 Subject: [PATCH 15/15] remove duplicated --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index f0e1169..095da2b 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,6 @@ This is a living repository — nothing is set in stone! If you're member of Met - [Engineering Principles](./docs/engineering-principles.md) - [JavaScript Guidelines](./docs/javascript.md) - [Migrations Best Practices](./docs/migrations-guidelines.md) -- [Secure Development Lifecycle Policy](./docs/sdlc.md) - [Pull Requests Guide](./docs/pull-requests.md) - [Secure Coding Guidelines](./docs/secure-coding-guidelines.md) - [Secure Development Lifecycle Policy](./docs/sdlc.md)