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

[WIP] add cypress #356

Merged
merged 7 commits into from
Sep 26, 2018
Merged

[WIP] add cypress #356

merged 7 commits into from
Sep 26, 2018

Conversation

timdeschryver
Copy link
Collaborator

@timdeschryver timdeschryver commented Sep 23, 2018

What:

Introduces cypress to run our e2e tests.

  • cypress config and npm scripts are added
  • protractor tests are rewritten
  • data-testid attributes are added in the html to use in the tests, prettier also formatted these files

Need to clear out

  • Is there a way to run the version build with npm run build:prod?
  • If this is OK what are we going to do with protractor?
  • Add to CI?

Issue number: N/A

package.json Outdated
"cy:run": "cypress run",
"precy:ci": "npm run build",
"cy:ci": "npm-run-all --parallel --race start:ci \"cy:run -- --config=baseUrl=http://localhost:4000\"",
"start:ci": "angular-http-server --path ./dist -p 4000 --silent"
Copy link
Owner

Choose a reason for hiding this comment

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

Could we prefix everything with cy: so that it is obvious that they belong together?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes!

@tomastrajan
Copy link
Owner

@timdeschryver looks very cool! Can we also run it as a part of npm run ci ?

@timdeschryver
Copy link
Collaborator Author

@timdeschryver looks very cool! Can we also run it as a part of npm run ci ?

We definitely can, but wanted to hear your thoughts on this, as well as the bullet points under "Need to clear out".

Copy link
Owner

@tomastrajan tomastrajan left a comment

Choose a reason for hiding this comment

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

Hi Tim!
I would add this to run with npm run ci but also kept the protractor as is so people see example of both, we can document that a bit in README.md (like a small note that they should choose and use just one).

Also how should I understand

Is there a way to run the version build with npm run build:prod?
what is the version build?

package.json Show resolved Hide resolved
@timdeschryver
Copy link
Collaborator Author

timdeschryver commented Sep 24, 2018

Is there a way to run the version build with npm run build:prod?

To build the app I tried using npm run build:prod but then the translations wouldn't come through because it searches the translations with

return new TranslateHttpLoader(
    http,
    `${environment.i18nPrefix}/assets/i18n/`,
    '.json'
  );

Thus it can't find the translations.

If I build the app with --deploy-url /angular-ngrx-material-starter/ --base-href /angular-ngrx-material-starter flags, I can't seem to run the app which is normal. The angular-ngrx-material-starter is needed for the github pages.

If you need more info, ask away.

@tomastrajan
Copy link
Owner

@timdeschryver could cypress be configured so it also run with the context path of /angular-ngrx-material-starter/ ?

Also...

I can't seem to run the app which is normal.

Can you please elaborate a bit more why this is the case?
I have no experience with cypress so maybe I am missing obvious things...

@timdeschryver
Copy link
Collaborator Author

This has nothing to do with Cypress but with "hosting" the application yourself.
For example with http-server or angular-http-server.

I'll take another look later to see if it's possible. In the past I wasn't succesful in finding a solution 😅

@tomastrajan
Copy link
Owner

This could work?

// cypress-server.js
const express = require('express');
const app = express();
app.use('/angular-ngrx-material-starter', express.static(__dirname + '/dist'));
app.listen(4200);

@timdeschryver
Copy link
Collaborator Author

I'll try it later today and keep you posted.

@timdeschryver
Copy link
Collaborator Author

timdeschryver commented Sep 24, 2018

You are a hero @tomastrajan 😄

@timdeschryver
Copy link
Collaborator Author

It should work with the last commit but I don't like it that we have to do the following in the cypress tests. Also I think this might crash the cy:open command.

cy.visit('/#about');

Another solution would be to not use the prod flag, or to change the localization paths if we're running the cypress ci tests. I think to do the latter we would have to introduce a new environment?

@tomastrajan
Copy link
Owner

We're using hash router strategy, also maybe `/#/about could work?

As for new env / config, I think it would make sense to define CI which doea prod build (angular.json configuration) and can have separate environment.ci.ts with no context paths which makes things easier, just have to think about running prod.build.on Travis CI so we can deploy app built.with context paths onto GH pages.

@timdeschryver
Copy link
Collaborator Author

/#/about works.

With giving it a second look with a fresh head I think we don't need a another env file. I didn't notice we were also hashing the router in dev mode. Our cypress tests runs fine in dev mode and prod mode, so I don't see a need to "complicate" things and add another config.

So I think all the things we needed to clear out are done:

  • we are building our app in prod mode and using an express server to run the app
  • we've added it to the ci
  • we're going to use protractor and cypress side by side to give a good comparison

If you're OK with this setup I can add it to the README.

@tomastrajan
Copy link
Owner

@timdeschryver yes, I think it is great like this, please update the README.md and we can merge this :)

@tomastrajan tomastrajan merged commit 6c3e781 into master Sep 26, 2018
@timdeschryver timdeschryver deleted the wip/cypress branch September 26, 2018 09:00
@singhcharanjit
Copy link

I added a new module, added new folder under i18n folder and also added TranslateModule.forChild() in imports of new module. Somehow it is not loading translation for new module.

What am I missing?

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

3 participants