Skip to content
This repository was archived by the owner on Dec 17, 2018. It is now read-only.

Adding protractor configuration #57

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Adding protractor configuration #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eecc717
Adding protractor configuration
el-davo Jul 18, 2017
2c4b24d
Updating jasmine-spec-reporter to 4.1.1
el-davo Jul 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions commands/initial/templates/e2e/example.e2e-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import {examplePo} from './page-objects/example.po';

describe('Example Page', () => {

beforeEach(() => examplePo.go());

it('should welcome user to the example page', () => {
expect(examplePo.welcome.getText()).toBe('Welcome');
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not generally a fan of default tests. I'd even like to get rid of the ones added to scaffolded unit tests but they're good sanity checks.

For this, it seems that anytime a library is generated, the dev has to remove this test--is that true? If it is, can we remove it and keep the rest of the setup?

Copy link

Choose a reason for hiding this comment

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

I know what you mean but they come with an advantage and at a cost:
The cost is that what you want to get rid of, the example app/spec/whatever.
The advantage is that your setup is already ready to go, you just need to overwrite the one test case you wanted to get rid off.
But I actually understand that for some who is like generating the 30th component he doesn't want to delete the example stuff all over again. That's why I would suggest adding a flag to every of your generating-component/directive/service/pipe script that is boolean and is named "--with--example" (or --without--example).

If you use something like rc then you can even use a configuration file that behaves like I described above (CLI > configuration > ...)

});
});
12 changes: 12 additions & 0 deletions commands/initial/templates/e2e/page-objects/example.po.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import {by, element, browser} from 'protractor';

class ExamplePo {

welcome = element(by.className('welcome'));

go() {
browser.get('/');
}
}

export const examplePo = new ExamplePo();
12 changes: 12 additions & 0 deletions commands/initial/templates/e2e/tsconfig.e2e.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"extends": "../tsconfig.json",
"compilerOptions": {
"outDir": "../out-tsc/e2e",
"module": "commonjs",
"target": "es5",
"types": [
"jasmine",
"node"
]
}
}
1 change: 1 addition & 0 deletions commands/initial/templates/examples/example.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<div class="welcome">Welcome</div>
Copy link
Owner

Choose a reason for hiding this comment

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

What does this HTML page do? Does it act as the index.html-type page where the tests are run?

Copy link
Contributor Author

@el-davo el-davo Jul 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The intent here was that we have a default example, much like the angular-cli. That was also the intent of the default test below. I can remove if not needed

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, we can scaffold out the skeleton of it and let devs add their own code. I think of how the CLI scaffolds out your initial components & HTML...it annoys me that everytime I create an app I have to remove all those files.

8 changes: 6 additions & 2 deletions commands/initial/templates/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
"prebuild": "rimraf dist out-tsc",
"start": "webpack-dev-server --open --config ./webpack/webpack.dev.js",
"tagVersion": "np --no-publish",
"test": "node ./tasks/test"
"test": "node ./tasks/test",
"pree2e": "webdriver-manager update",
"e2e": "protractor"
Copy link
Owner

Choose a reason for hiding this comment

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

Can protractor be run via a JS API? That way devs could run npm test e2e or npm run e2e...using npm test just keeps all of the testing in a single paradigm, which some might find nice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If i understand correctly you want to run the protractor tests along with the unit tests? I think that would cause a few issues

  • Protractor tests are very expensive to run. and would take a lot more time to run then unit tests.
  • When the user clicks save the tests most likely would not have finished executing before the next save
  • I think running protractor tests on save would be very annoying for the developer as it would be opening real browsers, basically robbing control from the user until the tests completed.
  • Keeping the tests in separate scripts is the recommended approach of the angular-cli

Copy link
Owner

Choose a reason for hiding this comment

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

Oh, sorry, I didn't mean along with unit tests. I do understand that protractor testes can be time-consuming.

What I meant was would we be able to leverage the current tasks/test.js file to execute the e2e tests? That way we keep the tests running all in one file.

Copy link

Choose a reason for hiding this comment

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

@gonzofish A seperated script for e2e tests is always better.
The reason lies within the fact that normally you would to do the following steps for a library (e.g. on a CI machine or during local development) and you wanna have a dedicated task for everyone of them:

  1. Build: npm run build
  2. Unit Test : npm run test
  3. serve (local) resp. deploy (CI): npm run serve/deploy
  4. e2e test: npm run e2e

The last step should have at least the possibility to specify the selenium server used ( e.g. localhost:4444 for local development and any other running selenium server when on CI).

So it's always better to have two scripts/tasks for the two different types of test, due to their needed tasks that should in between (serve resp. deploy)

Copy link
Owner

Choose a reason for hiding this comment

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

@nvdnkpr thanks for your continued participation in this project!

wouldn't being able to run npm test e2e satisfy the same thing as npm run e2e? We can update the test script to understand the arguments passed to it/add a configuration file so that the Selenium server port can be customized.

Does that satisfy your concerns?

Copy link

Choose a reason for hiding this comment

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

It's less about my concerns but more about simplicity. You can of course do what you describe but that somehow pulls all the test related parameters into your ./tasks/test script.

But NPM enables one to append configuration by appending it after a double dash after run command e.g. npm run e2e -- --seleniumServerAddress ADDRESS which gets resolved to protractor --seleniumServerAddress ADDRESS which then overwrites the default seleniumServerAddress (or the one from the configuration) with the new value ADDRESS.
(That's why most of the tools have the precedence in the following order: CLI > configuration > user-dotfile > default value)

This powermove of NPM enables you (the developer) to just run npm run e2e locally to run the tests (in this case: default) selenium server. In your CI environment however you then run npm run e2e -- --seleniumServerAddress ADDRESS to adjust your build script to the CI environment.

},
"keywords": [],
"author": "",
Expand Down Expand Up @@ -46,7 +48,7 @@
"html-webpack-plugin": "^2.19.0",
"istanbul-instrumenter-loader": "^1.2.0",
"jasmine-core": "2.5.2",
"jasmine-spec-reporter": "2.5.0",
"jasmine-spec-reporter": "^4.1.1",
"karma": "1.2.0",
"karma-chrome-launcher": "^2.0.0",
"karma-coverage-istanbul-reporter": "^1.3.0",
Expand All @@ -57,6 +59,7 @@
"node-sass": "^4.1.1",
"np": "^2.12.0",
"phantomjs-prebuilt": "^2.1.7",
"protractor": "^5.1.2",
"raw-loader": "^0.5.1",
"rimraf": "^2.5.3",
"rollup": "0.43.0",
Expand All @@ -67,6 +70,7 @@
"semver": "5.3.0",
"source-map-loader": "^0.1.5",
"style-loader": "^0.13.1",
"ts-node": "^3.2.0",
"tslint": "^4.0.2",
"tslint-loader": "^3.3.0",
"typescript": "~2.2.1",
Expand Down
39 changes: 39 additions & 0 deletions commands/initial/templates/protractor.conf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
const {SpecReporter} = require('jasmine-spec-reporter');
Copy link
Owner

Choose a reason for hiding this comment

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

Just like Webpack & Karma configs, this configuration should be extensible by devs using the library. So if they place a protractor.conf.js file in their configs directory, this config can be altered.

We'd need to identify the set of attributes the dev could override and which ones angular-librarian should be in charge of.

const WebpackDevServer = require('webpack-dev-server');
const webpack = require('webpack');
const config = require('./webpack/webpack.dev');

const PORT = 9001;

exports.config = {
allScriptsTimeout: 5000,
specs: [
'./e2e/**/*.e2e-spec.ts'
],
capabilities: {
'browserName': 'chrome'
Copy link
Owner

Choose a reason for hiding this comment

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

I know I'm being picky, but these quotes aren't needed, right? If not, I like to keep attribute names unquoted unless necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops yes agree with you here. will remove

},
directConnect: true,
baseUrl: `http://localhost:${PORT}/`,
framework: 'jasmine',
jasmineNodeOpts: {
showColors: true,
defaultTimeoutInterval: 30000
},
beforeLaunch() {
return new Promise(resolve => {
let compiler = webpack(config);
compiler.plugin('done', resolve);
new WebpackDevServer(compiler, {
disableHostCheck: true
}).listen(PORT, 'localhost');
});
},
onPrepare() {
require('ts-node').register({
project: 'e2e/tsconfig.e2e.json'
});

jasmine.getEnv().addReporter(new SpecReporter({spec: {displayStacktrace: true}}));
}
};
1 change: 0 additions & 1 deletion commands/initial/templates/webpack/webpack.dev.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ module.exports = webpackCommon('dev', {
devtool: 'cheap-module-eval-source-map',
entry: {
app: [ examplePath('example.main') ],
scripts: [],
Copy link
Owner

Choose a reason for hiding this comment

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

We need to keep the scripts attribute in webpack.dev.js

Copy link
Contributor Author

@el-davo el-davo Jul 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Webpack reports the following for me when trying to run it with scripts as an empty array

WebpackOptionsValidationError: Invalid configuration object. Webpack has been initialised using a configuration object that does not match the API schema.
 - configuration.entry should be one of these:
   object { <key>: non-empty string | [non-empty string] } | non-empty string | [non-empty string] | function
   The entry point(s) of the compilation.
   Details:
    * configuration.entry['scripts'] should be a string.
    * configuration.entry['scripts'] should not be empty.
    * configuration.entry['scripts'] should be one of these:
      non-empty string | [non-empty string]
    * configuration.entry should be a string.
    * configuration.entry should be an array:

I'm not sure what to do about this one, but it looks like a misconfiguration?

Copy link
Owner

Choose a reason for hiding this comment

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

Oh really? Interesting. Maybe that was an update. We'll keep it out for now...I might have even made that change, I can't remember 😉

vendor: [ webpackUtils.srcPath('vendor') ],
styles: [ examplePath('styles.scss') ]
},
Expand Down