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

[ci] Fix broken workflows #13931

Merged
merged 10 commits into from Aug 6, 2021
Merged

[ci] Fix broken workflows #13931

merged 10 commits into from Aug 6, 2021

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Aug 5, 2021

Why

fix ci broken

How

the broken on ios relates to cocoapods with incremental_installation which installer.pod_project might be nil if no new project generated.

the broken on android:

  • [detox] upgrade detox to 18.20.1 to support RN 0.64
  • [detox] allow local server cleartext traffic that i disallowed from release build during RN upgrade
  • [detox] fix building error with react-native-screens
  • [react-native] cherry-pick upstream patch which prevent *.so not packed in aar.

Test Plan

ci passed

 - upgrade detox to 18.20.1 to support rn 0.64
 - allow local server cleartext traffic that i disallow from release
   build during rn upgrade
@Kudo Kudo requested a review from tsapeta as a code owner August 5, 2021 13:53
@expo-bot expo-bot added the bot: passed checks ExpoBot has nothing to complain about label Aug 5, 2021
@@ -103,9 +103,9 @@ jobs:
id: pods-cache
with:
path: 'apps/bare-expo/ios/Pods'
key: ${{ runner.os }}-bare-expo-pods-${{ hashFiles('apps/bare-expo/ios/Podfile.lock') }}
key: ${{ runner.os }}-bare-expo-pods-v2-${{ hashFiles('apps/bare-expo/ios/Podfile.lock') }}
Copy link
Member

Choose a reason for hiding this comment

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

If you're sure it's cache issue, I would just wait for it to be invalidated instead of changing the keys 😉 However, I wonder why because the lock has changed after upgrade 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the root cause is installer.pods_project be null. according to this, bare-expo enabled incremental_installation make it possibly happened. that's why i tried not to use old cache.

do you know how to invalidate cache? according the GH document, we should wait 7 days.

btw GH actions is abnormal now, waiting for they fixing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can reproduce the error locally by pod install twice. (which incremental_installation takes effect)
so i reverted the cache key change and further check if installer.pod_project is not nil.

Copy link
Member

Choose a reason for hiding this comment

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

Afaik there is no way to immediately invalidate the cache, but I meant that any change in the lock should make it work 🙂 Also, there is a limit of 5GB for caches per repository, which I believe we achieve pretty quickly so older caches are for sure deleted in a few days.

@Kudo Kudo requested a review from brentvatne as a code owner August 5, 2021 19:16
@Kudo Kudo changed the title [bare-expo] Fix test-suite ci broken [ci] Fix broken workflows Aug 5, 2021
@@ -38,7 +38,7 @@ if (!Array.isArray(jestPreset.transformIgnorePatterns)) {

// Also please keep `testing-with-jest.md` file up to date
jestPreset.transformIgnorePatterns = [
'node_modules/(?!(jest-)?react-native|@react-native-community|expo(nent)?|@expo(nent)?/.*|react-navigation|@react-navigation/.*|@unimodules/.*|unimodules|sentry-expo|native-base|react-native-svg)',
'node_modules/(?!((jest-)?react-native|@react-native(-community)?)|expo(nent)?|@expo(nent)?/.*|react-navigation|@react-navigation/.*|@unimodules/.*|unimodules|sentry-expo|native-base|react-native-svg)',
Copy link
Member

Choose a reason for hiding this comment

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

i imagine the purpose of this is to handle https://www.npmjs.com/org/react-native 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cpojer published some @react-native prefixed packages and landed on RN 0.64, e.g. @react-native/polyfiils

@Kudo Kudo merged commit 24bce42 into master Aug 6, 2021
@Kudo Kudo deleted the @kudo/fix-ci-test-suites branch August 6, 2021 10:33
FelipeACP pushed a commit to FelipeACP/expo that referenced this pull request Sep 18, 2021
# Why

fix ci broken

# How

the broken on ios relates to cocoapods with `incremental_installation` which `installer.pod_project` might be nil if no new project generated.

the broken on android:
  - [detox] upgrade detox to 18.20.1 to support RN 0.64
  - [detox] allow local server cleartext traffic that i disallowed from release build during RN upgrade
  - [detox] fix building error with react-native-screens
  - [react-native] cherry-pick upstream patch which prevent *.so not packed in aar.

# Test Plan

ci passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants