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

With sequelize example #899

Closed

Conversation

mschipperheyn
Copy link
Contributor

With-sequelize example.

Highlights:

  • Creates docker mysql container
  • Creates database
  • Seeds database
  • Displays database contents on homepage

I can move this example to master if you prefer.

@mschipperheyn mschipperheyn mentioned this pull request Jan 27, 2019
Merged
5 tasks
Copy link
Owner

@jaredpalmer jaredpalmer left a comment

Choose a reason for hiding this comment

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

This is epic! You rock! Some little nits here and there.

examples/with-sequelize/README.md Outdated Show resolved Hide resolved
examples/with-sequelize/src/Home.js Outdated Show resolved Hide resolved
examples/with-sequelize/src/server.js Show resolved Hide resolved
examples/with-sequelize/src/sequelize/index.js Outdated Show resolved Hide resolved
@mschipperheyn
Copy link
Contributor Author

@jaredpalmer I have a question. I wonder if I should not perhaps add a test to promote api testing. The only thing is that I found that globalSetup and globalTeardown are not supported. Which is what I use to start a database server and force the creation of the table structure one time only. I test serially against a single database to prevent issues with various test creating and removing data and creating weird errors (I prefer integration testing against mock database testing).

I know some people like to use some wizardry to create a new database process with various ports to keep the parallel testing in place. I never tried it.

@elliottjro
Copy link
Collaborator

elliottjro commented Feb 25, 2020

@mschipperheyn for db setup, I usually put it into the package.json scripts as a setup and just add it as a requirement before running jest:

"scripts": {
  "unit": "node ./jest-setup/migrate-db.js && node ./jest-setup/seed-db.js && npm run test"
}

where migrate-db.js

const { spawn } = require('child-process-promise')

const run = async () => {
  const url = process.env.TEST_DB_URL || 'mysql://root@localhost:3306/database-test'
  try {
    await spawn('./node_modules/.bin/sequelize', ['db:migrate', `--url=${url}`], { stdio: 'inherit' })
    console.log('Migration success')
  } catch (e) {
    console.error('migration failed')
    process.exit(1)
  }
  process.exit(0)
}
run()

If you like, you can modify this PR (I know its quite old now, sorry for the delay) or I can just merge it in as is and you can make another PR when you have time to add this in?

@elliottjro elliottjro self-requested a review February 25, 2020 19:12
Copy link
Collaborator

@elliottjro elliottjro left a comment

Choose a reason for hiding this comment

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

Looks like all review change requests has been addressed

@elliottjro elliottjro removed the stale label Feb 26, 2020
@nimaa77
Copy link
Collaborator

nimaa77 commented Mar 13, 2020

@elliottjro hello, why you don't merge this 😎?

@fivethreeo fivethreeo changed the base branch from next to finch September 21, 2020 22:31
@fivethreeo
Copy link
Collaborator

Could you do this against the finch branch?

@fivethreeo fivethreeo closed this Oct 4, 2020
@fivethreeo
Copy link
Collaborator

Um, finch branch is now canary, can you do this against canary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants