## Summary
I introduced `xcbeautify` inside `make run-ios` command which would sometimes skip spitting crucial logs to the terminal.
In cases where the `react-native-cli` would prompt me of a port being used by another metro server, those prompts would also never reach me since they were piped into `xcbeautify` essentially giving me the impression that `make run-ios` is still doing something when it was actually hung.
This is how `react-native-community/cli` looks for `xcbeautify` ->
https://github.com/react-native-community/cli/blob/main/packages/cli-platform-apple/src/commands/buildCommand/buildProject.ts#L139
This commit fixes the problem by exporting an env variable making `react-native-cli` aware that we have `xcbeautify` installed and this way we now get the full log output we were used to.
Here we remove `yarn install` from `make component-test` and `make component-test-watch` commands.
We also fix an extra `@` present ahead of `@scripts/check-metro-shadow-process.sh` inside `component-test-watch` command.
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
`getEnvWithDefault.nix` prints a trace message in console when a default value is not set.
In this commit we add a check to see if default value matches the value of that env var.
Otherwise we get log messages like these when running `make run-ios`
```
trace: getEnvWithDefault IOS_STATUS_GO_TARGETS
(default: ios/arm64;iossimulator/amd64): ios/arm64;iossimulator/amd64
trace: getEnvWithDefault IOS_STATUS_GO_TARGETS
(default: ios/arm64;iossimulator/amd64): ios/arm64;iossimulator/amd64
```
We also fix a tiny mistake in `make run-ios` status-go target by updating the delimiter to be a semi-colon instead of a comma.
In this commit we build iOS `status-go` for both targets `amd64` and for `arm64` otherwise we get random failures when running `make run-ios`.
Maybe this was an impact of Xcode Upgrade Or RN Upgrade or MacOS Upgrade, it is not clear at the moment.
This will increase the time it took for iOS to be built locally, but it is better to be slow rather than have random build failures.
- make test still exists, so if you have been using it, as well as make
test-watch, they should all work exactly the same.
- [Changed] As part of the check stage, Jenkins will run Lint and Unit Tests
in parallel. Integration Tests run later because running them at the same
time as Unit Tests caused errors.
- [Added] "make unit-test" and "make unit-test-watch" run unit tests only.
Watching all unit tests is faster now because we ignore integration tests and
we only compile shadow-cljs :mock target once. We are at approximately 10-15s
to re-run all unit tests after saving a watched file, depending on your
hardware. If you change mocks.js_dependencies.cljs, you must re-run the make
target.
- [Added] "make integration-test" and "make integration-test-watch" run
integration tests only. These are much slower than the unit tests.
Fixes https://github.com/status-im/status-mobile/issues/18112
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.
I can't remember the number of times I have had to ask developers to run `make run-ios | xcbeautify` when debugging iOS build failures and they do not have `xcbeautify` installed on their environment.
`xcbeautify` helps make `xcodebuild` output more readable and to identify problems quickly.
This commit adds `xcbeautify` to iOS shell and to `make run-ios` so that we get readable output by default.
The metro terminal no longer needs to have `android` target anymore.
I had to do this in https://github.com/status-im/status-mobile/pull/17241
This commit sets the target of metro terminal back to `clojure`.
I tested building `android` and `iOS` on my MacOS and on my linux machines and found no side effect.
Now metro terminal is fast again.
This commit does many things :
- Upgrade `react-native ` to `0.72.5`
- Upgrade `react-native-reanimated` to `3.5.4`
- Upgrade `react-native-navigation` to `7.37.0`
- `ndkVersion` has been bumped to `25.2.9519653`
- `cmakeVersion` has been bumped to `3.22.1`
- `kotlinVersion` has been bumped to `1.7.22`
- `AGP` has been bumped to `7.4.2`
- `Gradle` has been upgraded to `8.0.1`
- Android `CompileSDK` and `TargetSDK` have been bumped to 33
- `@react-native-async-storage/async-storage` has been upgraded to `1.19.3`
- `@walletconnect/client` has been nuked
- some of the old `react-native-reanimated` code has been nuked
- `react-native-keychain` fork has been replaced with `8.1.2`
- On Android we are currently relying on `Hermes` Engine.
- On iOS we are currently relying on `JSC`
- We are not enabling new architecture for now (I have plans for that in the future) ref: https://github.com/status-im/status-mobile/issues/18138
IOS only PR : https://github.com/status-im/status-mobile/pull/16721
Android only PR : https://github.com/status-im/status-mobile/pull/17062
- `make run-metro` now has a target of `android` which was `clojure` earlier, this will increase the time it takes to start metro terminal but this is needed otherwise you will get a nasty error while developing for android locally.
The design team has now decided to keep iPhone 13 as the baseline standard instead of iPhone 11 Pro.
This commit updates the docs on pixel perfection and starting guide.
We also update the default simulator to iPhone 13 for `make run-ios`
Many times devs run
`make component-test` or `make component-test-watch` when there is already a metro or clojure terminal running on their system.
This causes weird behaviour and it is advised to not run these commands together.
This commit prevents that and shows a warning.
We've often seen cases of devs attempting to change dependencies outside a nix-shell and run into weird side effects
This commit stops them from :
- updating pods outside a nix shell
- updating node deps outside a nix shell
This commit also cleanup unused scripts in package.json and adds a fake comment script.
Since the `default` shell doesn't have Ruby, the `nix-update-gems`
target would incorrectly use the system Ruby instead of the one from Nix.
Signed-off-by: Jakub Sokołowski <jakub@status.im>
Change the make "lint" target default behavior to NOT show clj-kondo warnings.
In the CI, I kept the same behavior, i.e. show all warnings and errors
simultaneously.
Motivation:
When devs run make lint, most of the time, they don't want to see a long list of
warnings. Their focus is on the errors. Additionally, the majority of devs in
the mobile team see clj-kondo warnings in their editors of choice already.
We (some of us) believe the editor feedback/warnings are sufficiently noisy.
Add the following somewhere in your config files if you want to see
warnings.
export CLJ_LINTER_PRINT_WARNINGS=true
Disable :fn-deprecated warnings from the Closure compiler for the mobile
shadow-cljs target during development. The compiler warnings are no longer
necessary, because we're managing deprecation via clj-kondo.
Without the compiler setting's change, every code reload will generate warnings
for each and every deprecated call in the repo, which can quickly grow to the
hundreds or more, thus making the terminal output horrendous to understand.
Also set clj-kondo's "fail-level" to "error" (the default is "warning").
This commit removes the dependency of the IOS Target for `show-ios-devices` command.
We no longer need the IOS target because the default shell contains the `ios-deploy` library, which is used by this command already.
After the nixpkgs upgrade we started to have Xcode command line tools installation popup on each make test, dtrace-provider was failing meanwhile: #16356
It appeared, that it was failing before the upgrade with different issue: #16356
dtrace-provider is a dependency for detox, which was added for visiual tests in #14329
These tests don't run.
This build issues didn't cause any problems, because it was not obligatary.
See NODE_DTRACE_PROVIDER_REQUIRE option, which can enforce this requirement.
See #16356 for more details.
In this PR we disable detox dependency for now.
And also Visual Tests.
According to this line from the docs:
>The system-wide configuration file sysconfdir/nix/nix.conf (i.e. /etc/nix/nix.conf),
>or $NIX_CONF_DIR/nix.conf if NIX_CONF_DIR is set. Values loaded in this file are not
>forwarded to the Nix daemon. The client assumes that the daemon has already loaded them.
https://nixos.org/manual/nix/stable/command-ref/conf-file.html#description
Our usage of `NIX_CONF_DIR` has been wrong for a while now.
The correct way of applying this config is using `NIX_USER_CONF_FILES`.
In addition the `extra-substituters` no longer exists in the docs.
Use of `trusted-substituters` is necessary according to:
>At least one of the following conditions must be met for Nix to use a substituter:
>
>- the substituter is in the trusted-substituters list
>- the user calling Nix is in the trusted-users list
https://nixos.org/manual/nix/stable/command-ref/conf-file.html#conf-substituters
Signed-off-by: Jakub Sokołowski <jakub@status.im>
For some unknown to me reason we are using a different Yarn call to
Shadow-cljs to generate the JSBundle for iOS builds, while the one
created by the Android derivation shoudl be exactly the same.
I'm changing the target to just be `make jsbundle` while keeping aliases
referencing old naming, and moving things around in `nix` folder to
reflect the fact that the derivation is no longer Android-specific.
Also, crucially, I've changed the `import` in `index.js` to use the
`./result/index.js` path, since that's what Nix creates. I'm not sure if
this clashes with any developer workflow that takes place locally, so
I'd appreciate some testing from developers.
Depends on: https://github.com/status-im/status-jenkins-lib/pull/67
Signed-off-by: Jakub Sokołowski <jakub@status.im>
This commit does the following :
- Adds a command in makefile to show connected iOS devices via make `show-ios-devices`
- Developers can then copy their connected iPhone's Device Name and use it to deploy debug builds
on the device by `make run-ios-device DEVICE_NAME="their-device-name"`
- Enables automatic code signing ( only for Debug scheme )
The linter would fail if there were removed files, as it would
try to lint them but would not find them.
Similarly, untracked files would not be linted.
This commit changes the behavior so that untracked files are linted and
removed files are ignored, that way we can run it before committing if
there are unstaged changes that include removed/untracked files.
Siddarth Kumar
Icaro Motta
Ibrahem Khalil
flexsurfer
yqrashawn
Dmitri Akatov
Jakub Sokołowski
Ajay Sivan
Alex Tumanov
Anton Iakimov
Andrea Maria Piana
Rahul Pratap
Mohamed Javid
Jamie Caprani
Parvesh Monu
Omar Basem