feat(@angular/cli) override suite in the protractor config #8354
Conversation
There was a problem hiding this comment.
I think this is a nice feature, thanks for taking it on. It's important to verify it works and that we don't break it in the future, so can you add a e2e test please?
A good place would be in https://github.com/angular/angular-cli/tree/master/tests/e2e/tests/test.
|
@filipesilva started writing e2e and will append them to this pull request. |
aniruddhadas9
force-pushed
the
aniruddhadas9-patch-1
branch
from
fdecc4f to
eab1305
Compare
|
@filipesilva Have the changes met your requests? |
|
@aniruddhadas9 can you take a look at why the CI checks are failing for this? I would love to see this feature merged. |
|
@dallasacook sure, I will look into it and fix this build issue. |
|
@dallasacook @filipesilva, I have updated code and test case. I have manually tested all possible scenario to make sure everything works fine. The Travis job is failing due to committing message, I think it because of the merge commit happened when I updated code from the base and merged my code. Please let me know if anything else needs to be taken care off. |
There was a problem hiding this comment.
Besides the whitespace changes in tests/e2e/assets/1.0.0-proj/protractor.conf.js I think everything looks good!
CI is failing because of the commit message, yes... there's two things you need to do:
- have a single commit instead of 9 commits
- change the commit message to
feat(@angular/cli): override suite in the protractor config(notice the:)
You can do both with git rebase master -i.
| @@ -17,15 +17,16 @@ exports.config = { | |||
| jasmineNodeOpts: { | |||
There was a problem hiding this comment.
Can you remove the changes to this file? It's true that they are just harmless whitespace, but they are also unnecessary so we shouldn't change it without a good reason.
| @@ -34,6 +34,21 @@ export default function () { | |||
| .then(() => copyFile('./e2e/renamed-app.e2e-spec.ts', './e2e/another-app.e2e-spec.ts')) | |||
| .then(() => ng('e2e', '--specs', './e2e/renamed-app.e2e-spec.ts', | |||
| '--specs', './e2e/another-app.e2e-spec.ts')) | |||
| // Suites block need to be added in the protractor.conf.js file to test suites | |||
| .then(() => replaceInFile('protractor.conf.js', `allScriptsTimeout: 11000,`, | |||
There was a problem hiding this comment.
I think this is a good test. I tried to do this without the your PR and e2e broke with Error: Unknown test suite: app.
There was a problem hiding this comment.
Thank you :). Yap, I also tried same and some other combination and thought to add suites and then test it.
resolves: 807 Override suite in the protractor config. Can send in multiple suite by comma seperated values (ng e2e --suite=suite1.ts, suite2.ts). Issue link github.com/angular/issues/807 github.com/angular/pull/3551
aniruddhadas9
force-pushed
the
aniruddhadas9-patch-1
branch
from
40e7532 to
1e60634
Compare
|
@filipesilva Thanks for reviewing :) . Now merged all of them to one commit and removed formatting related changes in the |
|
@filipesilva its failing due to unable to connect to server now. |
|
@aniruddhadas9 thanks for taking the time to implement this feature! It should be on the next minor version, |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
feat(@angular/cli) override suite in the protractor config
resolves: 807
Override suite in the protractor config.
Can send in multiple suite by comma seperated values (ng e2e --suite=suite1.ts, suite2.ts).
Issue link
github.com//issues/807
github.com//pull/3551