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

Fix user e2e tests #194

Merged
merged 3 commits into from
Jun 6, 2022
Merged

Fix user e2e tests #194

merged 3 commits into from
Jun 6, 2022

Conversation

jbaez
Copy link
Contributor

@jbaez jbaez commented May 31, 2022

The e2e tests still don't seem to work due to migrations not running.

Changes:

  • update user.e2e tests to use new userService.createUser method
  • fix server typeorm command to use ORM config
  • update make test-e2e to re-create database volume every time
  • add User DTO
  • update auth.service and user.service to use User DTO
  • update CreateUserDto making optional properties that are optional

The e2e tests still don't seem to work due to migrations not running.

Changes:
- update user.e2e tests to use new `userService.createUser` method
- fix server `typeorm` command to use ORM config
- update make test-e2e to re-create database volume every time
- add User DTO
- update auth.service and user.service to use User DTO
- update CreateUserDto making optional properties that are optional
@alextran1502
Copy link
Contributor

Okay, I have figured out the issue. Here is how to solve it

  1. Create a file called entrypoint-test.sh in the server folder, below is the content
npm run typeorm -- migration:run && npm run test:e2e
  1. Modify the immich_server_test in the docker-compose.test.yml as follow:
  immich_server_test:
    image: immich-server-dev:1.9.0
    build:
      context: ../server
      dockerfile: Dockerfile
    entrypoint: ["/bin/sh", "./entrypoint-test.sh"]
    expose:
      - "3000"
    volumes:
      - ../server:/usr/src/app
      - /usr/src/app/node_modules
    env_file:
      - .env.test
    environment:
      - NODE_ENV=development
    depends_on:
      - redis
      - database
    networks:
      - immich_network_test
  1. Modify the typeorm command in the package.json as follow:
"typeorm": "node --require ts-node/register ./node_modules/typeorm/cli.js --config src/config/database.config.ts"
  1. Modify the migrations option in the database.config.ts as follow:
migrations: [__dirname + '/../migration/*.{js,ts}'],

Now run make test-e2e should work. The issue here is that we tell migration to look into the folder migration and look for any .js file as migration content, it only works after the system is built where the typescript files are compiled into javascript files. Here we tell the typeorm CLI to look directly into the source folder where there are no javascript files available. Step 4 ensures it takes into consideration of the .ts file as well

@jbaez
Copy link
Contributor Author

jbaez commented May 31, 2022

Okay, I have figured out the issue. Here is how to solve it

  1. Create a file called entrypoint-test.sh in the server folder, below is the content
npm run typeorm -- migration:run && npm run test:e2e
  1. Modify the immich_server_test in the docker-compose.test.yml as follow:
  immich_server_test:
    image: immich-server-dev:1.9.0
    build:
      context: ../server
      dockerfile: Dockerfile
    entrypoint: ["/bin/sh", "./entrypoint-test.sh"]
    expose:
      - "3000"
    volumes:
      - ../server:/usr/src/app
      - /usr/src/app/node_modules
    env_file:
      - .env.test
    environment:
      - NODE_ENV=development
    depends_on:
      - redis
      - database
    networks:
      - immich_network_test
  1. Modify the typeorm command in the package.json as follow:
"typeorm": "node --require ts-node/register ./node_modules/typeorm/cli.js --config src/config/database.config.ts"
  1. Modify the migrations option in the database.config.ts as follow:
migrations: [__dirname + '/../migration/*.{js,ts}'],

Now run make test-e2e should work. The issue here is that we tell migration to look into the folder migration and look for any .js file as migration content, it only works after the system is built where the typescript files are compiled into javascript files. Here we tell the typeorm CLI to look directly into the source folder where there are no javascript files available. Step 4 ensures it takes into consideration of the .ts file as well

Ah thanks! yeah, just did a quick test, from my PR the only thing missing is adding the.ts to the migrations 😄
I'll get that changed and also check if with that change it runs the migrations automatically (not sure why that stopped working). I'll be away until next week, but will get this updated as soon as possible.

- add missing `.ts` extension to migrations path
- update user e2e test for the new returned User resource
@jbaez
Copy link
Contributor Author

jbaez commented Jun 2, 2022

Yeah that missing .ts extension in the database config was the problem.
At the end there is no need of running the migration command before running the tests, the automatic migration is working fine 🙂
User e2e tests are passing now 👍

@jbaez
Copy link
Contributor Author

jbaez commented Jun 6, 2022

Merge conflict fixed, should be ready to be merged 🙂

@jbaez jbaez changed the title WIP fix user e2e tests Fix user e2e tests Jun 6, 2022
@alextran1502 alextran1502 merged commit b359dc3 into immich-app:main Jun 6, 2022
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.

None yet

2 participants