## Summary
PR made to add logs to help determine why integration test hangs on logout.
## Testing notes
No need for testing since we only ever add logs in test code here.
related to: #19256
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.
* 🚟 Add schema batch 2
* ⏬ Add maybe and optionals
* 🧑⚖️ Make theme a required prop
* 🍙 Fix misplaced square brackets that broke spec
* 🎨 Assume default theme and fix tests
- Fixes#18734
* ⬆️ Update schema and rebase
* 🧪 Update tests
* 🆙 Update progress bar value to be string or int
* 🔩 Tighten schema
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.
* Align docstring
* Create share-qr-code component
* Remove empty style file
* Implement common.share-qr-code including call to media server
* Add share-qr-code preview screen
* Use share-qr-code component in shell's share screen
* Add tests and some fixes
Force re-frame to stop warning about subscriptions in non-reactive contexts
while executing subscription tests (i.e. unit tests using the macro
test-helpers.unit/deftest-sub).
The net result? No more hundreds of useless warnings in the output of make test.
re-frame: Subscribe was called outside of a reactive context.
See: https://day8.github.io/re-frame/FAQs/UseASubscriptionInAJsEvent/
The warning is actually useful in production code, but in a subscription test we
already know we're never inside a reactive context.
Unshadows all remaining vars in status-mobile, including non
cljs.core/clojure.core ones. The only exceptions are cljs.core/type and
cljs.core/name (which happen quite often, so I'm not sure if it's worth
unshadowing them).
This commit implements the "Progress bar" component which is needed for wallet screen development.
Additionally, this commit adds a new test helper method "render-with-theme-provider" to test components in different themes.
Signed-off-by: Mohamed Javid <19339952+smohamedjavid@users.noreply.github.com>
- [x] Use correct icon to mark notifications as read
- [x] Add support for the disabled state in action drawer actions
- [x] Update quo2 preview `drawers` > `action-drawers`
- [x] Rewrite the action drawer component spec to use our test helpers
Fixes https://github.com/status-im/status-mobile/issues/14983
This commit makes the test-helpers.component namespace loadable in the REPL,
plus other changes that allow for a reasonably enjoyable RDD (REPL-Driven
Development) workflow.
Why? I want to be able to get instant feedback when I render a component with
the RN Testing Library (RNTL), and only once I'm satisfied with my findings is
when I proceed to write/update the tests. This nearly instant feedback loop is
only feasible using the ClojureScript REPL, and I'd rather not endure long
recompilation cycles.
Note that by REPL I mean connecting to the CLJS REPL of the Shadow-CLJS :mobile
target.
Essentially, this is what this commit does:
- [x] Allow the test-helpers.component namespace to be evaluated in the REPL.
This is now possible because I changed all functions that assumed js/jest
existed with a guard clause using the CLJS macro exists?. Without the
guard clauses, evaluating the namespace explodes due to stuff like
js/jest.useFakeTimers that fail in compile time (it's a syntax sugar
macro).
- [x] Change the family of functions to get the translation by text to either
translate using i18n/label or translate with the dummy prefix tx:,
depending if the code is running inside the Jest runtime or not.
- [x] Wrap remaining RNTL query functions, except for the find-* ones, since
they don't work at all outside the Jest runtime.
- [x] All wrapped functions support the original arguments supported by RNTL.
Arguments are always converted with clj->js.
- [x] All wrapped functions can optionally take a node (ReactTestInstance) as
their first argument, otherwise the global screen object will be used.
This is very important! See the explanation on section Doesn't RNTL
recommend using the screen object?
- [x] Update Shadow-CLJS preloads, so that (in development) you can fire off the
REPL and always be ready to call component test helpers. This is critical!
What else would be possible? Just an idea, but now that we can easily render
components using the same machinery provided by RNTL in the tests, we can
roughly implement Storybook's Play function
https://storybook.js.org/docs/react/writing-stories/play-function
Lesson learned: In the REPL, you may need to call
(re-frame.core/clear-subscription-cache!), otherwise you will experience
subscriptions returning the same value if their arguments are the same. For
example, I faced this while playing with the namespace
status-im2.contexts.communities.menus.community-options.component-spec. There
are better ways to solve this particular problem in the context of tests if we
use the tooling provided by day8.re-frame.test.
Doesn't RNTL recommend using the screen object? Indeed, it is recommended to use
the screen object instead of destructuring the results of RNTL render. It's just
easier and less error prone, but this only works reliably within the Jest
runtime, since it automatically cleans up rendered state after each test. When
using the REPL this is no longer the case, and I faced some errors, like Unable
to find node on an unmounted component, where RNTL would refuse to re-render
components, even if I explicitly unmounted them or called cleanup.
The only reliable solution I found was to store the result of render (a node)
and pass it to every subsequent call. This is not a workaround, it's officially
supported, but it's a tad less convenient. You can also not pass the node
reference and it should work most of the time.
Practical examples
Workflow suggestion: write your local experiments in the same namespace as the
component spec and within the comment macro. This way, you can have the Jest
watcher running and a REPL connected to :mobile, and they won't step on each
other. For the test watcher, I usually change quo2-core-spec or
status-im2.core-spec to only require what I'm interested, otherwise Jest
consumes way too many resources.
```clojure
;; Namespace quo2.components.colors.color-picker.component-spec
(h/test "color picker color changed"
(let [selected (reagent/atom nil)]
(h/render [color-picker/view {:on-change #(reset! selected %)}])
(h/fire-event :press (get (h/get-all-by-label-text :color-picker-item) 0))
(-> (h/expect @selected)
(.toStrictEqual :blue))))
(comment
(def selected (atom nil))
(def c (h/render [color-picker/view {:on-change #(reset! selected %)}]))
(h/fire-event :press (get (h/get-all-by-label-text c :color-picker-item) 0))
;; Options are passed down converted to JS types.
(h/debug c {:message "Rendering header"})
@selected ; => :blue
)
```
```clojure
;; Namespace quo2.components.tags.--tests--.status-tags-component-spec
(h/test "renders status tag with pending type"
(render-status-tag {:status {:type :pending}
:label "Pending"
:size :small})
(-> (h/expect (h/get-all-by-label-text :status-tag-pending))
(.toBeTruthy))
(-> (h/expect (h/get-by-text "Pending"))
(.toBeTruthy)))
(comment
(def c (render-status-tag {:status {:type :pending}
:label "Pending"
:size :small}))
(h/get-all-by-label-text c :status-tag-pending))
```
```clojure
;; Namespace status-im2.contexts.communities.menus.community-options.component-spec
(h/test "joined and muted community"
(setup-subs {:communities/my-pending-request-to-join nil
:communities/community {:joined true
:muted true
:token-gated? true}})
(h/render [options/community-options-bottom-sheet {:id "test"}])
(-> (h/expect (h/get-by-translation-text :unmute-community))
(.toBeTruthy)))
(comment
(setup-subs {:communities/my-pending-request-to-join nil
:communities/community {:joined true
:muted true
:token-gated? true}})
(def c (h/render [options/community-options-bottom-sheet {:id "test"}]))
(some? (h/get-by-translation-text c :invite-people-from-contacts)) ; => true
)
```
