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

feat(test): protractor integration, e2e test sample #94

Merged
merged 1 commit into from Jan 31, 2016

Conversation

filipesilva
Copy link
Contributor

Fix #72

@cironunes cironunes self-assigned this Dec 7, 2015
[ng test](https://github.com/angular/angular-cli/issues/70) is implemented.

To run the end-to-end protractor tests, just do `$(npm bin)/protractor`.
Copy link
Member

Choose a reason for hiding this comment

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

Please add a note saying that you need to have the server running (ng serve) to execute protractor tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Roger, it is done.

@cironunes
Copy link
Member

Thanks for the PR @filipesilva. I added a few comments, can you check?

@filipesilva filipesilva force-pushed the protractor-integration branch 2 times, most recently from 80f5c5c to d2b33e3 Compare December 7, 2015 14:36
@filipesilva
Copy link
Contributor Author

Let me know what you'd like to do about webdriver and I'll add that in.

In the meanwhile, do you have something else specific for me to do? Or should I just go over help-wanted issues?

@cironunes
Copy link
Member

@filipesilva you can take any issue that you feel like taking. The focus now is to build a cross-platform e2e testing suite for the project, but any help would be appreciated.

@filipesilva filipesilva force-pushed the protractor-integration branch 2 times, most recently from 5b4397f to df3e7c4 Compare December 7, 2015 19:14
@filipesilva
Copy link
Contributor Author

I moved the webdriver-manager update command to the readme. Anything else you think should be changed?

@cironunes
Copy link
Member

@filipesilva let's just figure out how we'll integrate this with ng test so we can get this merged.

@filipesilva
Copy link
Contributor Author

Hm... although ng test should be the umbrella command, unit tests and e2e have different use cases.

One would either run the unit tests once just to check everything is fine, or leave them running continuously while coding. In contrast, e2e tests are (in my experience) only ran once to check everything is fine.

Thus it might make sense to run e2e tests while running unit tests in single run, but it does not make sense to run e2e when unit tests are not in single run.

So to start off, I'd suggest that e2e tests are only ran explicitly, via ng test --e2e or something similar.

@IgorMinar
Copy link
Contributor

@filipesilva right on - see my comment: #86 (comment)

@@ -0,0 +1,9 @@
describe('<%= jsComponentName %> App', function() {

// #docregion redirect
Copy link
Contributor

Choose a reason for hiding this comment

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

docregion? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, a remnant of the devguide example, my bad :p

@IgorMinar
Copy link
Contributor

I'm not crazy about the manual steps we require, but we can fix that in follow up PRs.

How about renaming e2e extension to e2eSpec? Or is e2e clear enough? Maybe it is..

@juliemr do you have anything else you'd like to see in CLI when scaffolding the project/components?

@filipesilva
Copy link
Contributor Author

I'm partial to just e2e. I feel it's clear enough and doesn't lead users towards camelCase for their file names, eg my-component.e2e.ts vs my-component.e2eSpec.ts, and variations my_component, myComponent.

@cironunes
Copy link
Member

@filipesilva can you rebase the PR so we can get this merged?

@filipesilva
Copy link
Contributor Author

Apologies @cironunes, I had to return to docs for the last week.

This isn't ready to be merged yet though. While looking at the matchers that Igor was recommending, I found out since the *.e2e.ts files were being compiled to SystemJS module loading, that was causing all sorts of trouble when trying to use modules in general for protractor.

I will need to separate the e2e tests onto another folder, and provide it with tsconfig.json that uses CommonJS module loading.

@filipesilva filipesilva force-pushed the protractor-integration branch 2 times, most recently from dd508ad to 9deb8b3 Compare December 19, 2015 16:54
@filipesilva
Copy link
Contributor Author

I've updated the commit to reflect the major changes, so please review again.

Highlights:

  • added e2e folder besides src with own tsconfig.json
  • fixed imports
  • added page object

@cironunes
Copy link
Member

@filipesilva can you rebase this with the latest changes from master?

@filipesilva
Copy link
Contributor Author

Rebased and confirmed it was still working.

@cironunes
Copy link
Member

lgtm

@filipesilva filipesilva merged commit a0c26a3 into angular:master Jan 31, 2016
@filipesilva filipesilva deleted the protractor-integration branch June 13, 2016 16:12
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Protractor integration
4 participants