mirror of
https://github.com/status-im/status-react.git
synced 2025-01-09 10:42:53 +00:00
14 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Icaro Motta
|
96d98c62ed
|
chore(tests)_: Facilitate writing event tests (#20424)
Introduces a new macro deftest-event to facilitate writing tests for event handlers. Motivation came from the _problem of having to always extract event handlers as vars in order to test them_. Although the implementation of deftest-sub and deftest-event are similar, deftest-sub is critically important because it guarantees changes in one subscription can be caught by tests from all other related subscriptions in the graph (reference: PR https://github.com/status-im/status-mobile/pull/14472). This is not the case for the new deftest-event macro. deftest-event is essentially a way of make testing events less ceremonial by not requiring event handlers to be extracted to vars. But there are a few other small benefits: - The macro uses re-frame and "finds" the event handler by computing the interceptor chain (except :do-fx), so in a way, the tests are covering a bit more ground. - Slightly easier way to find event tests in the repo since you can just find references to deftest-event. - Possibly slightly easier to maintain by devs because now event tests and sub tests are written in a similar fashion. - Less code diff. Whether an event has a test or not, there's no var to add/remove. - The dispatch function provided by the macro makes reading the tests easier over time. For example, when we read subscription tests, the Act section of the test is always the same (rf/sub [sub-name]). Similarly for events, the Act section is always (dispatch [event-id arg1 arg2]). - Makes the re-frame code look more idiomatic because it's more common to define handlers as anonymous functions. Downside: deftest-sub and deftest-event are relatively complicated macros. Note: The test suite runs just as fast and clj-kondo can lint code within the macro just as well. Before: ```clojure (deftest process-account-from-signal-test (testing "process account from signal" (let [cofx {:db {:wallet {:accounts {}}}} effects (events/process-account-from-signal cofx [raw-account]) expected-effects {:db {:wallet {:accounts {address account}}} :fx [[:dispatch [:wallet/get-wallet-token-for-account address]] [:dispatch [:wallet/request-new-collectibles-for-account-from-signal address]] [:dispatch [:wallet/check-recent-history-for-account address]]]}] (is (match? expected-effects effects))))) ``` After ```clojure (h/deftest-event :wallet/process-account-from-signal [event-id dispatch] (let [expected-effects {:db {:wallet {:accounts {address account}}} :fx [[:dispatch [:wallet/get-wallet-token-for-account address]] [:dispatch [:wallet/request-new-collectibles-for-account-from-signal address]] [:dispatch [:wallet/check-recent-history-for-account address]]]}] (reset! rf-db/app-db {:wallet {:accounts {}}}) (is (match? expected-effects (dispatch [event-id raw-account]))))) ``` |
||
|
Icaro Motta
|
4fa9a27cd9
|
Rewrite contract tests to use new promise-based API (#19208) | ||
|
Icaro Motta
|
5d1e1f8005
|
Make integration tests more enjoyable to use (#19025)
This commit brings numerous improvements to integration tests. The next step will be to apply the same improvements to contract tests. Fixes https://github.com/status-im/status-mobile/issues/18676 Improvements: - Setting up the application and logged account per test is now done with an async test fixture, which is a very idiomatic way to solve this problem. No need anymore to write macros to wrap day8.re-frame.test/wait-for. The macros in test-helpers.integration will be removed once we apply the same improvements to contract tests. - Integration test timeouts can be controlled per test, with a configurable, global default (60s). - Now the integration test suite will fail-fast by default, i.e. a test failure short-circuits the entire suite immediately. This option can be overridden on a test-by-test basis. This improvement is very useful when investigating failures because the error will be shown on the spot, with no need to search backwards across lots of logs. - Noisy messages from re-frame can be silenced with a test fixture. We can silence even more in the future if we remove the hardcoded printf call from C++ on every signal and control it with Clojure. We can disable most logs as well with the more direct (status-im.common.log/setup "ERROR") at the top of tests.integration-test.core. We can make verbosity even more convenient to control, but I think this should be designed outside this PR. - Removed dependency on lib day8.re-frame/test for integration tests (see detailed explanation below). - Each call to (our) wait-for can customize the timeout to process re-frame event IDs passed to it. - Syntax is now flat, instead of being nested on every call to wait-for. You can now compose other async operations anywhere in a test. Notes: - promesa.core/do is essential in the integration test suite, as it makes sync & async operations play nice with each other without the developer having to promisify anything manually. - There are lots of logs related to async storage ("unexpected token u in JSON at position..."). This isn't fixed yet. Are we not going to use day8.re-frame.test? We don't need this library in integration tests and we won't need it in contract tests. Whether it will be useful after we remove it from integration and contract tests is yet to be seen (probably not). A few reasons: - The async model of promises is well understood and battle tested. The one devised in the lib is poorly understood and rigid. - There's basically no way to correctly integrate other async operations in the test, unless they can be fully controlled via re-frame events. For instance, how would you control timeouts? How would you retry in a test? How would forcefully delay an operation by a few seconds? These things are useful (to me at least) when developing integration/contract tests. - Every call to day8.re-frame.test/wait-for forces you to nest code one more level. Code readability suffers from that choice. - Have you ever looked up the implementation of wait-for? It's quite convoluted. One could say the source code is not that important, but many times I had to look it up because I couldn't understand the async model they built with their macro approach. The de facto primitive in JS for asynchronicity is promises, and we fully leverage it in this PR. - The lib has an interesting macro run-test-sync, but we have no usage for it. I used it in status-mobile for a while. At one point, all event unit tests for the Activity Center used it (e.g. commit 08fb0de7b09beec83e91567cbf2ff795cde39f3f), but I replaced them with the simpler pure function style. |
||
|
Icaro Motta
|
a14e228e3a
|
Fix: add back missing clj-kondo config files (#18472)
Fix our issues with auto-generated clj-kondo config files. - To be extra precise, we now exclude ".clj-kondo/*" from being processed by clojure-lsp clean-ns. - Formatted the .zprintrc file. Fixes https://github.com/status-im/status-mobile/issues/17947 |
||
|
Icaro Motta
|
0b4a1545ae
|
Fix component tests, upgrade Jest & friends, and a few other goodies (#18276)
Fix all component tests after the latest RN upgrade. Fixes https://github.com/status-im/status-mobile/issues/18157 Closes https://github.com/status-im/status-mobile/pull/18235 Dependency changes - Upgraded Jest: from 26.6.3 to latest 29.7.0. - Upgraded @testing-library/jest-native: from 5.3.0 to latest 5.4.3 - Upgraded @testing-library/react-native: from 11.5.4 to 12.4.2 - Removed explicit dependency on jest-circus, this is now the default test runner. - Removed explicit dependency on jest-environment-node. This is handled by the package manager. - Added jest-silent-reporter at version 0.5.0. ### Why component tests were failing? Many tests were failing because we were using RN Testing Library (RNTL) in an unreliable fashion. With the recent library upgrades, the unreliability was excerbated. Other times, the tests were incorrectly arranging data. ### with-redefs does not work with async code Generally speaking, with-redefs should not be used with async code, assume the worst. The scope of the macro will cease to exist by the time the async code runs. In many tests we were using with-redefs, then calling render, but for some components that use use-effect, JS timers, animations, etc it's unreliable and were the reason for failures. It's easy to reproduce too: ```clojure (defn foo [] :foo) (foo) ;; => :foo (with-redefs [foo (constantly :bar)] (foo)) ;; => :bar (js/setTimeout (fn [] (tap> [:calling-foo (foo)])) 100) ;; Taps [:calling-foo :foo] ;; As you would expect, when running without with-redefs, it prints :foo. ;; So far so good, but whatch what happens with async code: (with-redefs [foo (constantly :bar)] (js/setTimeout (fn [] (tap> [:calling-foo (foo)])) 100)) ;; Taps [:calling-foo :foo] ;; ====> PROBLEM: Taps :foo, not :bar as one might expect ``` ### Not waiting on wait-for When test-helpers.component/wait-for is used, subsequent assertions/etc should be done after the promise returned by wait-for is resolved. But remember to not perform side-effects inside the wait-for callback (check out the docs https://callstack.github.io/react-native-testing-library/docs/api#waitfor). Most, if not all of our usages of wait-for were not waiting. #### Improvement 1 - Silence Jest on demand If you need to re-run component tests frequently, you may want to reduce the output verbosity. By passing JEST_USE_SILENT_REPORTER=true to make component-test or make component-test-watch you will see a lot less noise and be able to focus on what really matters to you. #### Improvement 2 - Selectively focus/disable tests Because of our need to first compile CLJS to JS before running tests via Jest, we couldn't easily skip or focus on specific tests. From this commit onwards, we should never again have to change the list of requires in files core_spec.cljs. Commenting out required namespaces gives a bad DX because it causes constant rebasing issues. #### Improvement 3 - Translations now work as in prod code (but only English) Translations performed by *-by-translation-text can be done now without any workaround under the hood. The query functions are now linted just like i18n/label, which means static translation keywords must be qualified with :t/, which is good for consistency. |
||
|
Icaro Motta
|
439fdfa12c
|
Rename effects to :effects.*/* and move them to separate namespaces (#18047) | ||
|
Icaro Motta
|
c1dcd7a764
|
Introduce malli library (#17867)
This commit is the foundational step to start using malli (https://github.com/metosin/malli) in this project. Take in consideration we will only be able to realize malli's full power in future iterations. For those without context: the mobile team watched a presentation about malli and went through a light RFC to put everyone on the same page, among other discussions here and there in PRs. To keep things relatively short: 1. Unit, integration and component tests will short-circuit (fail) when inputs/outputs don't conform to their respective function schemas (CI should fail too). 2. Failed schema checks will not block the app from initializing, nor throw an exception that would trigger the LogBox. Exceptions are only thrown in the scope of automated tests. 3. There's zero performance impact in production code because we only instrument. Instrumentation is removed from the compiled code due to the usage of "^boolean js.goog/DEBUG". 4. We shouldn't expect any meaningful slowdown during development. **What are we instrumenting in this PR?** Per our team's agreement, we're only instrumenting the bare minimum to showcase 2 examples. - Instrument a utility function utils.money/format-amount using the macro approach. - Instrument a quo component quo.components.counter.step.view/view using the functional approach. Both approaches are useful, the functional approach is powerful and allow us to instrument anonymous functions, like the ones we pass to subscriptions or event handlers, or the higher-order function quo.theme/with-theme. The macro approach is perfect for functions already defined with defn. **I evaluated the schema or function in the REPL but nothing changes** - If you evaluate the source function, you need to evaluate schema/=> or schema/instrument as well. - Remember to *var quote* when using schema/instrument. - You must call "(status-im2.setup.schema/setup!)" after any var is re-instrumented. It's advisable to add a keybinding in your editor to send this expression automatically to the CLJS REPL, or add the call at the end of the namespace you are working on (similar to how some devs add "(run-tests)" at the end of test namespaces). **Where should schemas be defined?** For the moment, we should focus on instrumenting quo components, so define each function schema in the same namespace as the component's public "view" var. To be specific: - A schema used only to instrument a single function and not used elsewhere, like a quo component schema, wouldn't benefit from being defined in a separate namespace because that would force the developer to constantly open two files instead of one to check function signatures. - A common schema reused across the repo, like ":schema.common/theme" should be registered in the global registry "schema.registry" so that consumers can just refer to it by keyword, as if it was a built-in malli schema. - A common schema describing status-go entities like message, notification, community, etc can be stored either in the respective "src/status_im2/contexts/*" or registered globally, or even somewhere else. This is yet to be defined, but since I chose not to include schemas for them, we can postpone this guideline. |
||
|
Icaro Motta
|
6e897f0ea6
|
Migrate away from rf/defn and rf/merge (first step) (#17451)
This commit shows how to move away from rf/defn |
||
Ulises Manuel Cárdenas
|
0e36190516
|
Fix input padding & add blur and override-theme properties
Add with-let formatting style |
||
|
0f8ad69319
|
Nix/upgrade zprint 1.2.5 (#15113)
* nix: upgrade zprint from 1.2.4 to 1.2.5 To address issue described in: https://github.com/kkinnear/zprint/issues/273 Signed-off-by: Jakub Sokołowski <jakub@status.im> * chore: use zprint :multi-lhs-hang * refactor: re-format clojure using zprint 1.2.5 --------- Signed-off-by: Jakub Sokołowski <jakub@status.im> Co-authored-by: yqrashawn <namy.19@gmail.com> |
|||
|
Icaro Motta
|
915b8ebd9a
|
New component Selector > Filter (#14650) | ||
yqrashawn
|
9ded8bfe97
|
chore: tweak zprint conf (#14645) | ||
|
Icaro Motta
|
a8d392e4de
|
Redirect user to Activity Center to manage pending contact requests (#14610) | ||
|
yqrashawn
|
39e29cfb5a
|
feat: replace clj-fmt with zprint (#14520) |
