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

node-app nestjs e2e test module #1098

Closed
4 tasks done
xmlking opened this issue Feb 22, 2019 · 43 comments
Closed
4 tasks done

node-app nestjs e2e test module #1098

xmlking opened this issue Feb 22, 2019 · 43 comments
Labels
outdated scope: node Issues related to Node, Express, NestJS support for Nx type: feature

Comments

@xmlking
Copy link
Contributor

xmlking commented Feb 22, 2019

Prerequisites

Please answer the following questions for yourself before submitting an issue.
YOU MAY DELETE THE PREREQUISITES SECTION.

  • I checked the documentation and found no answer
  • I am running the latest version
  • I checked to make sure that this issue has not already been filed
  • I'm reporting the issue to the correct repository (not related to Angular, AngularCLI or any dependency)

Expected Behavior

ng g node-app api --framework=nestjs should generate both api and api-e2e modules

Current Behavior

only generate api module

Similar to webApps , we need e2e testing for node-apps
Proposal :
we can generate NestJS e2e module something like this https://github.com/xmlking/ngx-starter-kit/tree/develop/apps/api-e2e

and angular.json entry
https://github.com/xmlking/ngx-starter-kit/blob/develop/angular.json#L286

then we can do e2e tests with:

ng e2e api-e2e
ng e2e api-e2e --watch
@atul221282
Copy link

+1

@gperdomor
Copy link
Contributor

any update/plan about this?

@gperdomor
Copy link
Contributor

@FrozenPandaz @vsavkin any chance to fix this for Nx 8 release?

@xmlking
Copy link
Contributor Author

xmlking commented May 7, 2019

Thought for feedback:
Would it be better if we have e2e module within main app? For both angular and nestjs apps
This why, We can have fewer apps in angular.json

webapp 
    src
    e2e
backend
    src
    e2e

@gperdomor
Copy link
Contributor

@xmlking I prefer that too, is a cleaner solution... In fact, both angular and NestJS have e2e test in some folder inside the main project folder when are created with their official cli...

NestJS directory structure (created using nest new nests-app):

nestjs-app
├── README.md
├── nest-cli.json
├── node_modules
├── nodemon-debug.json
├── nodemon.json
├── package-lock.json
├── package.json
├── src
├── test ---> e2e tests resides here
├── tsconfig.build.json
├── tsconfig.json
└── tslint.json

Angular directory structure (created using ng new angular-app ):

angular-app
├── README.md
├── angular.json
├── e2e ---> e2e tests resides here
├── node_modules
├── package-lock.json
├── package.json
├── src
├── tsconfig.json
└── tslint.json

xmlking added a commit to xmlking/ngx-starter-kit that referenced this issue May 10, 2019
merged api, api-e2e projects and wepapp and webapp-e2e projects

nrwl/nx#1098
@xmlking
Copy link
Contributor Author

xmlking commented May 10, 2019

in the above commit, I merged e2e projects with corresponding parent projects (both for angular/cypress and nestjs)

they are working fine with following commands

ng e2e api
ng e2e webapp

ng lint api will lint files in src and e2e

wish cypress builder support outputPath option like @angular-devkit/build-angular, so that we can keep cypress.json relative paths free (e.g. ../../../dist/apps/...)

@gperdomor
Copy link
Contributor

@xmlking can you send a PR to this repo? 🙏

@Jordan-Hall
Copy link
Contributor

I prefer the existing approach. In a mono repo, wouldn't it be better to split these out of the common application because companies tend to do smoke screen tests now and then on the live system periodically?

@xmlking
Copy link
Contributor Author

xmlking commented May 11, 2019

One of the reason I am motivated to merge e2e and main src in single project is , direct e2e dependency on main app code, https://github.com/xmlking/ngx-starter-kit/blob/develop/apps/api/e2e/src/app.e2e-spec.ts#L4

@Jordan-Hall
Copy link
Contributor

Jordan-Hall commented May 11, 2019

@xmlking that's because of how the tests are done using jest and are only an issue with the node applications. In the example, the "end2end" testing is actually done more on a unit test/integration perspective rather than the real "end2end"

I think they some confusion over the testing. This to me doesn't feel like end2end and it should be done using jest personally. When it comes to the frontend you use protector or cypress that files to a URL. rather than what happening in the example which is you building a local version of the site to test. the node API.

@FrozenPandaz
Copy link
Collaborator

For starters, I don't think this is in scope for 8.0.0. We're focused on making sure repackaging is a good experience and migrating to the new architect API. But I would like to join the discussion to share my thoughts and work towards a plan for 8.x

My main motivation for having an e2e project for a node-app is for repositories that contain only backend code. Such an e2e project would likely use something like supertest (as the nest-cli does) via jest to actually make a request to a running server and check the response. Cypress would likely not be a good choice. It would probably not be generated by default as being full-stack is a major benefit but there would be an option for it. We could probably create a builder composing around the @nrwl/node:execute and @nrwl/jest:jest builders.

As for having the e2e tests within the application projectRoot, I feel more strongly against this. My reason is that e2e tests could depend on more than just a single application. For example, login might be a separate app altogether which can be part of the e2e test suite.

@gperdomor
Copy link
Contributor

gperdomor commented May 15, 2019

@FrozenPandaz well, I think that the node-app should be able to be tested no matter which approach is decided... The nest template provides a good structure for e2e tests using supertest like you mention... I don't know how complex would be using the same e2e tests generated by the the nest-cli

@Jordan-Hall
Copy link
Contributor

I'm with @FrozenPandaz can test e2e on node application is excellent. But my main reservation is it going to the main application root.

I like splitting the responsibility of my backend code, for example, having a microservice that deals with database connections and another for authentication etc.

@gperdomor
Copy link
Contributor

as Nx 8 was released, how can we deal with nestjs e2e test? this will be added in the future?

@gperdomor
Copy link
Contributor

ping @FrozenPandaz @vsavkin

@vsavkin
Copy link
Member

vsavkin commented Jun 18, 2019

As Jason mentioned the @nrwl/node:execute and @nrwl/jest:jest builders should be sufficient to implement an e2e test project.

Run:

ng g @nrwl/nest:app api
ng g @nrwl/node:app api-e2e

Next, rename the test target in the api-e2e project into e2e, and write tests using say supertest.

Having said that, I think it is a good idea to generate an e2e test project for both express and nest apps. Could anyone write a proposal of what it will look like for both express and nest projects/

@vsavkin vsavkin self-assigned this Jun 18, 2019
@vsavkin vsavkin added this to the nx9 milestone Jun 20, 2019
@gperdomor
Copy link
Contributor

@vsavkin what do you mean with rename the test target in the api-e2e project into e2e?

@gperdomor
Copy link
Contributor

@vsavkin omit my last question... I understand 👍

I delete the src folder in api-e2e and update the angular.json like that:

    ...
    "api-e2e": {
      "root": "apps/api-e2e",
      "sourceRoot": "apps/api-e2e/src",
      "projectType": "application",
      "prefix": "api-e2e",
      "schematics": {},
      "architect": {
        "lint": {
          "builder": "@angular-devkit/build-angular:tslint",
          "options": {
            "tsConfig": [
              "apps/api-e2e/tsconfig.app.json",
              "apps/api-e2e/tsconfig.spec.json"
            ],
            "exclude": ["**/node_modules/**", "!apps/api-e2e/**"]
          }
        },
        "e2e": {
          "builder": "@nrwl/jest:jest",
          "options": {
            "jestConfig": "apps/api-e2e/jest.config.js",
            "tsConfig": "apps/api-e2e/tsconfig.spec.json"
          }
        }
      }
    }

Basic test:

// some-test-file.spec.ts
import { Test, TestingModule } from '@nestjs/testing';
import * as request from 'supertest';
import { AppModule } from '../../api/src/app/app.module'; // <----- fails

describe('AppController (e2e)', () => {
  let app;

  beforeEach(async () => {
    const moduleFixture: TestingModule = await Test.createTestingModule({
      imports: [AppModule]
    }).compile();

    app = moduleFixture.createNestApplication();
    await app.init();
  });

  it('/ (GET)', () => {
    return request(app.getHttpServer())
      .get('/')
      .expect(200)
      .expect({ message: 'Welcome to api!' });
  });
});

running npm run e2e -- --project=api-e2e works fine but npm run lint -- --project=api-e2e fails with this error: library imports must start with @nx-platform/

Also glad to see this will be improved and supported in Nx 9

@tfalvo
Copy link

tfalvo commented Jul 4, 2019

I followed @vsavkin advice, and as @gperdomor, I created a new app api-e2e, and rename test target to e2e.

Then, in the spirit of Nx modularization, I moved code of my NestJs api application to at least 2 libs for instance :

  • api-feature (with controller/service for one feature)
  • api-application (with app.module.ts only)

It's clear, that I have a very specific lib with app.module.ts, but it's not a real problem for me. However, if we don't want to create this lib, we could consider to leave AppModule very small, with only main imports. So it's easy to recreate it both in app and app-e2e.

Finally, I imported theses 2 libs into both api and api-e2e application.
And... it works like a charm !

ng e2e api-e2e
ng lint api-e2e

Really interested to receive your feedback/comments.

Thank you guys for your sharing, and all your great job !
I'm a huge fan of Nx. 👍

@gperdomor
Copy link
Contributor

@tfalvo can you send us the link of the repo?

@vsavkin vsavkin removed their assignment Aug 7, 2019
@johannesschobel
Copy link
Contributor

the solution described by @vsavkin and @gperdomor works perfectly.. However, a more "clean" approach would be to bootstrap it like @tfalvo described in his post.. will dive into this later, i guess :)

cheers to all of you for making nrwl such an awesome experience!

@vsavkin vsavkin added scope: node Issues related to Node, Express, NestJS support for Nx and removed discussion labels Dec 4, 2019
@nrwl nrwl deleted a comment from github-actions bot May 29, 2020
@FrozenPandaz
Copy link
Collaborator

Hi, sorry about this.

This was mislabeled as stale. We are testing ways to mark not reproducible issues as stale so that we can focus on actionable items but our initial experiment was too broad and unintentionally labeled this issue as stale.

@vsavkin vsavkin removed this from the Nx 10 milestone Jul 8, 2020
@lucasmonstrox
Copy link

@tfalvo do you think having e2e for nestjs is not tricky and stable?

@tfalvo
Copy link

tfalvo commented Aug 31, 2020

Hi @luqezman, could you precise your question please. e2e with NestJS or e2e with NestJS + NX, or just e2e vs simple unit test??...

@beeman
Copy link
Contributor

beeman commented Sep 12, 2020

Having said that, I think it is a good idea to generate an e2e test project for both express and nest apps. Could anyone write a proposal of what it will look like for both express and nest projects/

Hey @vsavkin , @DominikPieper and I took some time to go over this, and we came up with the proposal below.

A (manual) implementation of this structure with a Nest API can be found here.

The only downside to this proposal so far is that AppModule requires a path-based import:

image

Which makes WebStorm complain:

image

It would be great if we could have a way to address this, without having to change too much (if anything at all) on the side of the application.

Looking forward to your feedback :)


Proposal: Create e2e projects for Node apps in Nx Workspace

Backend applications in an Nx Workspace (generated with the @nrwl/node, @nrwl/express and @nrwl/nest plugins) don't generate e2e application, unlike frontend projects.

The reasons behind this is that when e2e tests that are added to a frontend project, they will generally also test the backend.

However, in some cases it makes sense to add an e2e test for Node-based projects, for instance if there is only backend code in your workspace.

Solution

Existing projects

Create a new schematic to @nrwl/node plugin called e2e-app. The schematic takes as an argument the name of an existing project (of type 'application') in the workspace:

nx generate @nrwl/node:e2e-app api

Running this schematic creates a new project in the workspace file called api-e2e, and add 2 architects: e2e and lint.

The structure of the generated app will be very similar to the -e2e apps that are generated for the frontend.

Having this schematic being standalone allows people to opt in to having an e2e test for a backend project, for instance in an existing Nx Workspace.

New projects

Additionally, when generating a new project using @nrwl/node:app, @nrwl/express:app or @nrwl/nest:app, the user should be able to opt-in to creating this -e2e application.

For instance, using this command:

nx generate @nrwl/nest:app api --e2e

Will result in a structure like this:

apps/api/
apps/api-e2e/

@khalilou88
Copy link
Contributor

Hi all, I don't think we need a e2e for api projects. For me its only integrations tests that should live inside the api projects.
with the actual solution we don't need to import AppModule from another project (e2e).
If you choose to use a e2e project, I think we need to use an url for the api and don't import code from it.

@AliYusuf95
Copy link
Contributor

I agree with @khalilou88,
e2e test shouldn't import the files from other projects, @beeman this is the way to solve that 😄 .
We need a solution similar to the one that used in frontend e2e projects, which is serve the frontend app then execute the tests files. As what @nrwl/cypress:cypress builder does.

// example configuration
// ... node-e2e
"e2e": {
    "builder": "@nrwl/jest:jest", // <--- update/create new builder to accept `devServerTarget` option
    "options": {
        "jestConfig": "apps/node/jest.config.js",
        "devServerTarget": "backend:serve"  // <--- this is what we need, run another node application here (backend) 
    },
    "configurations": {
        "production": {
            "devServerTarget": "backend:serve:production"
        }
    }
},

From e2e files, we can use the API URL which supertest allows.

@JoA-MoS
Copy link
Contributor

JoA-MoS commented Mar 26, 2021

I am questioning if making a separate e2e project makes sense for the type of tests that @beeman and @gperdomor and the nestjs example https://docs.nestjs.com/fundamentals/testing#end-to-end-testing. The way that these tests are run are fundamentally different than a cypress test. In cypress tests the tests do not need to have any insight of the inner workings of the applications. In the nestjs example e2e the tests themselves are creating the server and this is what allows you to mock different aspects of the nestjs application.

In cypress, the cypress tooling provides that functionality through intercept and fixtures. This logic lives outside your core application, cypress interacts with the browser api not your application.

Now I could definitely see the benefit of an e2e test project and configuration like @AliYusuf95 mentions but these would likely be more for true System Integration Tests. Where they run against the devServerTarget and the specs never reference the apps AppModule. The reason I say a System Integration Test is I am unaware of a built in functionality like cypress provides to mock the connection to your database or other api. This could be done with some fore thought in your engineering and use a flag to swap out the injected providers for mock providers but I am wondering if there is a better way. Swapping the services/providers for mock providers is what they are doing in the e2e example on nestjs.

@JoA-MoS
Copy link
Contributor

JoA-MoS commented Mar 26, 2021

Alright, looks like nock might be a good solution for nodejs network request mocking. https://www.npmjs.com/package/nock

@beeman
Copy link
Contributor

beeman commented Mar 26, 2021

@JoA-MoS I'd love to see another way to do it if there is.

I've been working on most of my projects with the suggestion I described above, and it worked very well.

Having this way of testing (basically, a single collection of tests that are firing queries to the backend - for me it's not relevant if it's called e2e or integration test), helped me and the teams I worked with a lot in getting proper tests for our backend.

If there are ways to optimize / improve I'd definitely be interested in giving that a shot.

Happy to connect if you like to chat about it :)

@JoA-MoS
Copy link
Contributor

JoA-MoS commented Mar 31, 2021

Thanks @beeman I will try your approach and the other options and see what makes most sense. How did you get around your open question in your post (the linting issue)?

If I have more questions I will message you on slack

@Jonathan002
Copy link

I noticed this question was raised before Nx 8 release. It appears we are in Nx @nrwl/schematics
8.12.11. Is this supported yet?

@npwork
Copy link

npwork commented Dec 26, 2021

+1 for this feature

@markush97
Copy link

+1 this would be really appreciated

@rkristelijn
Copy link

rkristelijn commented May 30, 2022

I've tried all above situations, however it did not bring what I needed. We use nx for a NestJs backend and an Angular frontend. In the frontend everything runs fine with Cypress and mocking the request to the backend. But we also want a similar test scenario for the NestJs Backend including a database.

I resorted back to start-server-and-test package and npm scripts for now, my solutions looks like this:

# create a dummy backend-e2e project
nx g @nrwl/node:app backend-e2e
  • remove all contents of the src folder from the project backend-e2e
  • write a simple test
import * as fetch from 'isomorphic-fetch';

describe('FormsData', () => {
  const server = 'http://localhost:3333/graphql';
  it('should do something', async () => {
    const result = await fetch(server, {
      method: 'POST',
      headers: { 'Content-Type': 'application/json' },
      body: JSON.stringify({ query:
        `query { __schema { queryType { name } }  }`
      })
    })

    expect(result).toBeTruthy();
  });
});
  • npm install --save-dev start-server-and-test
  • add to scripts in `package.json:
  // [...]
  "scripts": {
    // [...]
    "pree2e:server": "docker-compose up -d",
    "e2e:server": "nx run backend:serve --skip-nx-cache",
    "e2e:test": "nx run backend-e2e:e2e",
    "e2e": "start-server-and-test e2e:server 'http-get://localhost:3333/graphql?query={ __schema { queryType { name } } }' e2e:test"
  }

It got me unstuck for now, looking forward to a nx-build-in solution

@GregOnNet
Copy link

Hi,

thanks for the hint, creating an e2e-Project for a Nest Project.

  • After creating the e2e-app, I need to bootstrap the Nest-App.
  • Therefore, I need to import AppModule.
  • Then, eslint complains that importing a app is not allowed:
    Imports of apps are forbidden eslint @nrwl/nx/enforce-module-boundaries
    

Is there a way to allow the app-Import in this special case?

image

@exsesx
Copy link

exsesx commented Sep 25, 2022

So, any updates? What'd be the right way to add e2e tests?

@beeman
Copy link
Contributor

beeman commented Sep 25, 2022

So, any updates? What'd be the right way to add e2e tests?

I've been using something like this since I wrote it and it works great for me:
#1098 (comment)

@jdutheil
Copy link

Still nothing on that ? I can't find a way to write e2e tests like in NestJs doc, that's pretty annoying right now

@kodze
Copy link

kodze commented Dec 22, 2022

Hello :)

Is there a plan to implement this feature in the next releases?

Thanks for your work!

@AgentEnder
Copy link
Member

This is fixed by #14805

@github-actions
Copy link

github-actions bot commented May 1, 2023

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated scope: node Issues related to Node, Express, NestJS support for Nx type: feature
Projects
None yet
Development

No branches or pull requests