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

Allow specifying the RN version in test-turbo-module.sh #162

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

Johennes
Copy link
Collaborator

No description provided.

@@ -179,7 +186,7 @@ create_library() {
--type module-new \
--example $example_type \
--local false \
"$base" > /dev/null
"$base"
Copy link
Collaborator Author

@Johennes Johennes Nov 12, 2024

Choose a reason for hiding this comment

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

I've removed the redirection here because it makes the script exit silently on error without any way to find out what went wrong.

As an example, [email protected] cannot generate for [email protected] apparently:

$ ./scripts/test-turbo-modules.sh -r 0.73.0 tm-test
ℹ️  Starting my-test-library
-- Creating library my-test-library with create-react-native-library@latest
ℹ Using [email protected] for the example
⠴ Copying files

The generated example app doesn't have community CLI installed
node:internal/process/promises:288
            triggerUncaughtException(err, true /* fromPromise */);
            ^

AssertionError [ERR_ASSERTION]: The generated example app doesn't have community CLI installed
    at Object.create [as handler] (/Users/jm/.npm/_npx/e106e660d51959d2/node_modules/create-react-native-library/lib/index.js:557:31) {
  generatedMessage: false,
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: '=='
}

Node.js v18.19.1

@Johennes Johennes requested a review from jhugman November 12, 2024 12:50
Copy link
Owner

@jhugman jhugman left a comment

Choose a reason for hiding this comment

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

This looks good! What are you going to do with it?

@Johennes
Copy link
Collaborator Author

Johennes commented Nov 12, 2024

I wanted to see if I can set up a small test matrix that runs the script on the CI for different RN and builder-bob versions so that we can get some visibility into which combinations currently work and be notified when one of them breaks. Right now I'm basically running into the various externally triggered breakages by coincidence.

Do you think that would be helpful?

@jhugman
Copy link
Owner

jhugman commented Nov 12, 2024

I wanted to see if I can set up a small test matrix that runs the script on the CI for different RN and builder-bob versions so that we can get some visibility into which combinations currently work and be notified when one of them breaks. Right now I'm basically running into the various externally triggered breakages by coincidence.

Do you think that would be helpful?

Getting this to run on CI would be amazing, if nothing else to get that area of the code tested in a more systematic way.

Having an early warning system for breakage based on the shifting sands of RN and CRNL would be super cool.

Would you be triggering this to run every time CRNL makes a release? or daily or something even cleverer?

I do worry that if CRNL breaks our build, that would stop us landing code elsewhere in the project—so I'd likely suggest that these tests trigger if a PR makes changes in the crates/ubrn_cli directory, or the test-turbo-modules.sh file itself.

@Johennes
Copy link
Collaborator Author

I was thinking about nightly for a start just to get a sense of stability and then go from there? Definitely agree that we should limit its impact on PRs given that external factors could break it.

@Johennes Johennes merged commit 33fb8c6 into main Nov 13, 2024
5 checks passed
@Johennes Johennes deleted the johannes/rn-version branch November 13, 2024 07:11
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