Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

feat: reseed db before tests #7747

Merged
merged 7 commits into from
Mar 21, 2023

Conversation

aditya-mitra
Copy link
Collaborator

Summary

Using an app setting testModeForceRefresh = true, the database can be seed in between tests.
Here, this setting has been used in user.test.ts

References

Closes #7726

Checklist

  • If this PR is still a WIP, convert to a draft
  • When this PR is ready, mark it as "Ready for review"
  • ensure all checks pass
  • Changes have been manually QA'd
  • Changes reviewed by at least 2 approved reviewer

QA Steps

List any additional steps required to QA the changes of this PR, as well as any supplemental images or videos.

@aditya-mitra aditya-mitra self-assigned this Mar 13, 2023
Copy link
Member

@HexaField HexaField left a comment

Choose a reason for hiding this comment

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

replicating this locally, I get "message": "Table 'etherealengine.analytics' doesn't exist",

My guess is this doesn't work for multiple tests.

@@ -33,7 +33,9 @@ export default (app: Application): void => {
promiseReject = reject
})

app.setup = async function (...args: any) {
app.setup = async function (...args) {
const testModeForceRefresh = app.get('testModeForceRefresh')
Copy link
Member

Choose a reason for hiding this comment

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

rather than setting this manually, we can rely on appConfig.testEnabled, such that it is automatically applied every time createFeathersExpressApp is run

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried to do this initially but encountered some problems. DB is also reseeded before tests (in npm run pretest) which requires process.exit(0) here. Using appConfig.testEnabled will have some problems.

Copy link
Member

Choose a reason for hiding this comment

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

Ahh I see. Let's use the appConfig instead of feathers internal state. We can add a new configuration option with the desired functionality.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added this change!

@aditya-mitra
Copy link
Collaborator Author

replicating this locally, I get "message": "Table 'etherealengine.analytics' doesn't exist",

My guess is this doesn't work for multiple tests.

@HexaField Can you please tell me the steps you followed to replicate this?

This is because I am testing this on a fresh database and not getting any error.
I also added this reseed in a few more tests in packages/server-core to check if it works across different test cases. It seemed to be working.

image

Can you please recheck it once?

@HexaField
Copy link
Member

HexaField commented Mar 15, 2023

@HexaField Can you please tell me the steps you followed to replicate this?

Can you please recheck it once?

@aditya-mitra
I have copied your code exactly, and added

  if (appConfig.testEnabled) 
    app.set('testModeForceRefresh', true)

to createFeathersExpressApp in line 146 to ensure it reseeds before every test

This is the result:

[1678916225094] ERROR: Duplicate key name 'identity_provider_id'
    component: "server-core:sequelize"
    err: {
      "type": "DatabaseError",
      "message": "Duplicate key name 'identity_provider_id'",

@aditya-mitra
Copy link
Collaborator Author

@HexaField I found the error (it was a problem with a test) and have fixed it.

```ts
  if (appConfig.testEnabled) 
    app.set('testModeForceRefresh', true)

to createFeathersExpressApp in line 146 to ensure it reseeds before every test

I have also added the setting to reseed db in appConfig itself using above method. All tests will now reseed db before running their tests.

@HexaField
Copy link
Member

@HexaField I found the error (it was a problem with a test) and have fixed it.

I have also added the setting to reseed db in appConfig itself using above method. All tests will now reseed db before running their tests.

Fantastic, let's replace the usage of testModeForceRefresh with appConfig.testEnabled directly and this should be good to go

@aditya-mitra
Copy link
Collaborator Author

Fantastic, let's replace the usage of testModeForceRefresh with appConfig.testEnabled directly and this should be good to go

Done with these changes!

Copy link
Member

@HexaField HexaField left a comment

Choose a reason for hiding this comment

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

in server start.ts L27 and server-core seeder.tsx L63 we should add && !config.testEnabled to both conditionals

@cla-bot
Copy link

cla-bot bot commented Mar 21, 2023

Thank you for your pull request and welcome to the Ethereal Engine developer community!

We require contributors to sign our Copyright Assignment Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign the agreement at https://forms.gle/15ENsSAJGKf2ozvB7

The agreement has not been signed by users: @aditya-mitra.

After signing the agreement, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR :)

@HexaField HexaField self-requested a review March 21, 2023 03:23
@aditya-mitra
Copy link
Collaborator Author

@cla-bot check

@cla-bot
Copy link

cla-bot bot commented Mar 21, 2023

Checking that all contributors to this PR are verified.

@HexaField HexaField added this pull request to the merge queue Mar 21, 2023
@HexaField HexaField removed this pull request from the merge queue due to a manual request Mar 21, 2023
@HexaField HexaField merged commit e800d12 into EtherealEngine:dev Mar 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Research how feathers does it's tests and present and implement it in the user service
3 participants