Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(BSR) build(ios): fix build #7662

Merged
merged 5 commits into from
Feb 24, 2025
Merged

(BSR) build(ios): fix build #7662

merged 5 commits into from
Feb 24, 2025

Conversation

bebstein-pass
Copy link
Contributor

@bebstein-pass bebstein-pass commented Feb 11, 2025

Link to JIRA ticket: https://passculture.atlassian.net/browse/PC-XXXXX

Flakiness

If I had to re-run tests in the CI due to flakiness, I add the incident on Notion

Checklist

I have:

  • Made sure my feature is working on web.
  • Made sure my feature is working on mobile (depending on relevance : real or virtual devices)
  • Written unit tests native (and web when implementation is different) for my feature.

Best Practices

Click to expand These rules apply to files that you make changes to. If you can't respect one of these rules, be sure to explain why with a comment. If you consider correcting the issue is too time consuming/complex: create a ticket. Link the ticket in the code.
  • In the production code: remove type assertions with as (type assertions are removed at compile-time, there is no runtime checking associated with a type assertion. There won’t be an exception or null generated if the type assertion is wrong). In certain cases as const is acceptable (for example when defining readonly arrays/objects). Using as in tests is tolerable.
  • Remove bypass type checking with any (when you want to accept anything because you will be blindly passing it through without interacting with it, you can use unknown). Using any in tests is tolerable.
  • Remove non-null assertion operators (just like other type assertions, this doesn’t change the runtime behavior of your code, so it’s important to only use ! when you know that the value can’t be null or undefined).
  • Remove all @ts-expect-error and @eslint-disable.
  • Remove all warnings, and errors that we are used to ignore (yarn test:lint, yarn test:types, yarn start:web...).
  • Use gap (ViewGap) instead of <Spacer.Column />, <Spacer.Row /> or <Spacer.Flex />.
  • Don't add new "alias hooks" (hooks created to group other hooks together). When adding new logic, this hook will progressively become more complex and harder to maintain.
  • Remove logic from components that should be dumb.

Test specific:

  • Avoid mocking internal parts of our code. Ideally, mock only external calls.
  • When you see a local variable that is over-written in every test, mock it.
  • Prefer user to fireEvent.
  • When mocking feature flags, use setFeatureFlags. If not possible, mention which one(s) you want to mock in a comment (example: jest.spyOn(useFeatureFlagAPI, 'useFeatureFlag').mockReturnValue(true) // WIP_NEW_OFFER_TILE in renderPassPlaylist.tsx )
  • In component tests, replace await act(async () => {}) and await waitFor(/* ... */) by await screen.findBySomething().
  • In hooks tests, use act by default and waitFor as a last resort.
  • Make a snapshot test for pages and modals ONLY.
  • Make a web specific snapshot when your web page/modal is specific to the web.
  • Make an a11y test for web pages.

Advice:

  • Use TDD
  • Use Storybook
  • Use pair programming/mobs

@bebstein-pass
Copy link
Contributor Author

@nmajorfrances-pass est-ce que tu arrives à build avec ça ?

@mmeissonnier-pass tu avais ajouté la dépendance, est-ce que fonctionne encore chez toi ?

@bebstein-pass
Copy link
Contributor Author

en installant jdk pour le build Android, ça a cassé le build iOS 🤷 plus d'info dans le second commit

le build iOS est encore cassé sur ma machine, mais ça échoue plus tard dans le process, il faut que je regarde pourquoi

le build Android est cassé sur ma machine avec le proxy d'activé (problème de certificat)

en désactivant le proxy, le build échoue avec l'erreur suivante

'compileDebugJavaWithJavac' task (current target is 11) and 'compileDebugKotlin' task (current target is 17) jvm target compatibility should be set to the same Java version.

donc peut etre que ça casse le build Android, il faut peut etre que je ré-exporte des variables d'environnment nécessaire à Android

@bebstein-pass bebstein-pass marked this pull request as draft February 11, 2025 18:41
Copy link
Contributor

@nmajorfrances-pass nmajorfrances-pass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ca build chez moi

Before this commit
==================

When doing the following command

```sh
yarn ios:prod
```

It fails with the error

```txt
warn Package react-native-flipper contains invalid configuration: "dependency.platforms.ios.project" is not allowed. Please verify it's properly linked using "react-native config" command and contact the package maintainers about this.
info Found Xcode workspace "PassCulture.xcworkspace"
info No booted devices or simulators found. Launching first available simulator...
The file /nix/store/2y71rl5mnazwnpwpcq07xx97jl7sy5lh-apple-sdk-11.3/Applications/Simulator.app does not exist.
error Command failed: open /nix/store/2y71rl5mnazwnpwpcq07xx97jl7sy5lh-apple-sdk-11.3/Applications/Simulator.app --args -CurrentDeviceUDID EBDBAC65-9268-4D4A-8785-03A099743F7B
The file /nix/store/2y71rl5mnazwnpwpcq07xx97jl7sy5lh-apple-sdk-11.3/Applications/Simulator.app does not exist.
.
Error: Command failed: open /nix/store/2y71rl5mnazwnpwpcq07xx97jl7sy5lh-apple-sdk-11.3/Applications/Simulator.app --args -CurrentDeviceUDID EBDBAC65-9268-4D4A-8785-03A099743F7B
The file /nix/store/2y71rl5mnazwnpwpcq07xx97jl7sy5lh-apple-sdk-11.3/Applications/Simulator.app does not exist.

    at checkExecSyncError (node:child_process:890:11)
    at Object.execFileSync (node:child_process:926:15)
    at runOnSimulator (/Users/eb/Project/pass-culture/pass-culture-app-native/node_modules/@react-native-community/cli-platform-ios/build/commands/runIOS/index.js:219:28)
    at Object.runIOS [as func] (/Users/eb/Project/pass-culture/pass-culture-app-native/node_modules/@react-native-community/cli-platform-ios/build/commands/runIOS/index.js:144:14)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Command.handleAction (/Users/eb/Project/pass-culture/pass-culture-app-native/node_modules/@react-native-community/cli/build/index.js:111:9)
```

When looking for XCode path, with the following command

```sh
xcode-select --print-path
```

It outputs this

```txt
/nix/store/2y71rl5mnazwnpwpcq07xx97jl7sy5lh-apple-sdk-11.3
```

Because installing JDK througth DevBox set the following variable environment

```sh
DEVELOPER_DIR=/nix/store/2y71rl5mnazwnpwpcq07xx97jl7sy5lh-apple-sdk-11.3
```

XCode-select seems to display this variable when set

The following test command

```sh
DEVELOPER_DIR=toto xcode-select --print-path
```

Displays

```txt
toto
```

After this commit
=================

```sh
xcode-select --print-path
```

```txt
/Applications/Xcode.app/Contents/Developer
```
avant, ça me semblait nécessaire

maintenant, j'ai l'impression que ça cause plus de problèmes que ça n'en résoud
use Nix for dependencies

use DevBox when needed to pin dependencies with a specific version

sometimes some dependencies have some issues with DevBox
@bebstein-pass
Copy link
Contributor Author

bebstein-pass commented Feb 18, 2025

avec le Proxy activé :

  • iOS XCode ✅
  • iOS CLI ✅
  • Android Studio ❌ chez moi, c'est pété sur master (pareil sur cette branche)
  • Android CLI ✅
  • test:unit ✅ (j'ai pas tout lancé, ça prends trop longtemps sur ma machine)

@bebstein-pass bebstein-pass marked this pull request as ready for review February 18, 2025 12:22
@bebstein-pass bebstein-pass merged commit ea83581 into master Feb 24, 2025
52 checks passed
@bebstein-pass bebstein-pass deleted the ios-build-fix branch February 24, 2025 10:52
nmajorfrances-pass pushed a commit that referenced this pull request Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants