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

Add tests to the app generated by create-react-admin with ra-data-fakerest #9578

Merged
merged 10 commits into from
Jan 23, 2024

Conversation

djhi
Copy link
Collaborator

@djhi djhi commented Jan 11, 2024

How To Test

make build-create-react-admin install

Then from outside the project monorepo, call:

[PATH TO MONOREPO]/node_modules/.bin/create-react-admin myadmin --data-provider ra-data-fakerest --auth-provider none --install npm
like:
./react-admin-next/node_modules/.bin/create-react-admin myadmin2 --data-provider ra-data-fakerest --auth-provider none --install npm

and run the tests:

cd myadmin
npm run test

@djhi djhi changed the title Add tests to the ra-data-fakerest generated app Add tests to the app generated by create-react-admin with ra-data-fakerest Jan 11, 2024
Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

I also have a few more important remarks.

I tested the 'published' version of the CLI (following the instructions in the Readme) and found out that:

  1. Running npm install to install the build version of the CLI locally fails, because of the following error: Could not resolve dependency: peer react@"^16.5.2 || ^17.0.0" from ink-select-input@4.2.2 (since we now require react 18) => Do we need to update to a newer version of ink-select-input?
  2. in the generated app, we have both a .gitignore and a gitignore file
  3. the generated Readme does not explain how to run the tests
  4. The test is failing in the generated project, because of a timeout (Test timed out in 5000ms) although the app is running properly locally 🤷‍♂️
  5. The tests seem to assume we chose an authProvider, but it's not mandatory. We should only include the test if an authProvider was selected IMO

@adguernier adguernier self-requested a review January 12, 2024 08:46
Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

Code looks good, but:

  • There is still a gitignore file generated in addition to .gitignore
  • Tests are still failing when run in the generated project (timeout after 5000 ms)

@djhi
Copy link
Collaborator Author

djhi commented Jan 17, 2024

Code looks good, but:

  • There is still a gitignore file generated in addition to .gitignore

FIxed

  • Tests are still failing when run in the generated project (timeout after 5000 ms)

Not sure what happen on your machine. It works fine on mine and on the CI (#9580)

@slax57
Copy link
Contributor

slax57 commented Jan 23, 2024

Tests are still failing when run in the generated project (timeout after 5000 ms)

Okay so I pushed the investigation further

  • Tests run fine with the parameter --auth-provider none
  • Tests fail (on my machine) with the parameter --auth-provider local-auth-provider (timeout after 5000 ms)
  • If I increase the test timeout to 6000 ms then it works

=> Hence I recommend increasing the timeout a bit by default, otherwise we risk having the test fail on the CI too 🤷

@slax57 slax57 added this to the 5.0.0 milestone Jan 23, 2024
@slax57 slax57 merged commit a2f273f into next Jan 23, 2024
11 checks passed
@slax57 slax57 deleted the create-react-admin-tests branch January 23, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants