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(@angular/cli): ng e2e defaults to random port #4753

Merged
merged 2 commits into from
Feb 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 1 addition & 7 deletions packages/@angular/cli/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const baseBuildCommandOptions: any = [
{ name: 'i18n-format', type: String },
{ name: 'locale', type: String },
{ name: 'extract-css', type: Boolean, aliases: ['ec'] },
{ name: 'watch', type: Boolean, aliases: ['w'] },
{ name: 'watch', type: Boolean, default: false, aliases: ['w'] },
{
name: 'output-hashing',
type: String,
Expand Down Expand Up @@ -60,12 +60,6 @@ const BuildCommand = Command.extend({
run: function (commandOptions: BuildTaskOptions) {
const project = this.project;

const additionalDefaults: any = {
watch: false
};

commandOptions = Object.assign({}, additionalDefaults, commandOptions);

// Check angular version.
Version.assertAngularVersionIs2_3_1OrHigher(project.root);

Expand Down
34 changes: 17 additions & 17 deletions packages/@angular/cli/commands/e2e.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
const SilentError = require('silent-error');

import { overrideOptions } from '../utilities/override-options';
import { CliConfig } from '../models/config';
import { ServeTaskOptions, baseServeCommandOptions } from './serve';
import { checkPort } from '../utilities/check-port';
const Command = require('../ember-cli/lib/models/command');


Expand All @@ -13,31 +15,27 @@ export interface E2eTaskOptions extends ServeTaskOptions {
elementExplorer: boolean;
}

export const e2eCommandOptions = baseServeCommandOptions.concat([
{ name: 'config', type: String, aliases: ['c'] },
{ name: 'specs', type: Array, default: [], aliases: ['sp'] },
{ name: 'element-explorer', type: Boolean, default: false, aliases: ['ee'] },
{ name: 'webdriver-update', type: Boolean, default: true, aliases: ['wu'] },
{ name: 'serve', type: Boolean, default: true, aliases: ['s'] }
]);


const E2eCommand = Command.extend({
name: 'e2e',
aliases: ['e'],
description: 'Run e2e tests in existing project',
works: 'insideProject',
availableOptions: e2eCommandOptions,
availableOptions: overrideOptions(
baseServeCommandOptions.concat([
{ name: 'config', type: String, aliases: ['c'] },
{ name: 'specs', type: Array, default: [], aliases: ['sp'] },
{ name: 'element-explorer', type: Boolean, default: false, aliases: ['ee'] },
{ name: 'webdriver-update', type: Boolean, default: true, aliases: ['wu'] },
{ name: 'serve', type: Boolean, default: true, aliases: ['s'] }
]), [
{ name: 'port', default: 0 },
{ name: 'watch', default: false },
]
),
run: function (commandOptions: E2eTaskOptions) {
const E2eTask = require('../tasks/e2e').E2eTask;
this.project.ngConfig = this.project.ngConfig || CliConfig.fromProject();

const additionalDefaults: any = {
watch: false
};

commandOptions = Object.assign({}, additionalDefaults, commandOptions);

const e2eTask = new E2eTask({
ui: this.ui,
project: this.project
Expand Down Expand Up @@ -72,7 +70,9 @@ const E2eCommand = Command.extend({
}
}

serve.run(commandOptions, rebuildCb)
checkPort(commandOptions.port, commandOptions.host)
.then((port: number) => commandOptions.port = port)
.then(() => serve.run(commandOptions, rebuildCb))
.catch(reject);
});
} else {
Expand Down
81 changes: 41 additions & 40 deletions packages/@angular/cli/commands/serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { baseBuildCommandOptions } from './build';
import { CliConfig } from '../models/config';
import { Version } from '../upgrade/version';
import { ServeTaskOptions } from './serve';
import { checkPort } from '../utilities/check-port';
import { overrideOptions } from '../utilities/override-options';

const SilentError = require('silent-error');
const PortFinder = require('portfinder');
Expand Down Expand Up @@ -60,53 +62,52 @@ const ServeCommand = Command.extend({
description: 'Builds and serves your app, rebuilding on file changes.',
aliases: ['server', 's'],

availableOptions: baseServeCommandOptions.concat([
{ name: 'live-reload', type: Boolean, default: true, aliases: ['lr'] },
{
name: 'live-reload-host',
type: String,
aliases: ['lrh'],
description: 'Defaults to host'
},
{
name: 'live-reload-base-url',
type: String,
aliases: ['lrbu'],
description: 'Defaults to baseURL'
},
{
name: 'live-reload-port',
type: Number,
aliases: ['lrp'],
description: '(Defaults to port number within [49152...65535])'
},
{
name: 'live-reload-live-css',
type: Boolean,
default: true,
description: 'Whether to live reload CSS (default true)'
},
{
name: 'hmr',
type: Boolean,
default: false,
description: 'Enable hot module replacement',
}
]),
availableOptions: overrideOptions(
baseServeCommandOptions.concat([
{ name: 'live-reload', type: Boolean, default: true, aliases: ['lr'] },
{
name: 'live-reload-host',
type: String,
aliases: ['lrh'],
description: 'Defaults to host'
},
{
name: 'live-reload-base-url',
type: String,
aliases: ['lrbu'],
description: 'Defaults to baseURL'
},
{
name: 'live-reload-port',
type: Number,
aliases: ['lrp'],
description: '(Defaults to port number within [49152...65535])'
},
{
name: 'live-reload-live-css',
type: Boolean,
default: true,
description: 'Whether to live reload CSS (default true)'
},
{
name: 'hmr',
type: Boolean,
default: false,
description: 'Enable hot module replacement',
}
]), [
{ name: 'watch', default: true },
]
),

run: function (commandOptions: ServeTaskOptions) {
const ServeTask = require('../tasks/serve').default;

const additionalDefaults: any = {
watch: true
};

commandOptions = Object.assign({}, additionalDefaults, commandOptions);

Version.assertAngularVersionIs2_3_1OrHigher(this.project.root);
commandOptions.liveReloadHost = commandOptions.liveReloadHost || commandOptions.host;

return checkExpressPort(commandOptions)
return checkPort(commandOptions.port, commandOptions.host)
.then((port: number) => commandOptions.port = port)
.then(() => autoFindLiveReloadPort(commandOptions))
.then((opts: ServeTaskOptions) => {
const serve = new ServeTask({
Expand Down
24 changes: 24 additions & 0 deletions packages/@angular/cli/utilities/check-port.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import * as denodeify from 'denodeify';

const SilentError = require('silent-error');
const PortFinder = require('portfinder');
const getPort = <any>denodeify(PortFinder.getPort);

PortFinder.basePort = 49152;


export function checkPort(port: number, host: string) {
return getPort({ port, host })
.then((foundPort: number) => {

// If the port isn't available and we weren't looking for any port, throw error.
if (port !== foundPort && port !== 0) {

Choose a reason for hiding this comment

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

I wonder why you did not just return the found port instead of throwing an error. This would help developers in case the port is already in use, I work with multiple projects at the same time and often find my self having to serve with a random port number. For example ng serve --port=2010

throw new SilentError(
`Port ${port} is already in use. Use '--port' to specify a different port.`
);
}

// Otherwise, our found port is good.
return foundPort;
});
}
12 changes: 12 additions & 0 deletions packages/@angular/cli/utilities/override-options.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
const cloneDeep = require('lodash/cloneDeep');

export function overrideOptions(original: any[], overrides: any[]) {
let copy = cloneDeep(original);
overrides.forEach(override => {
const option = copy.find((opt: any) => opt.name == override.name);
if (option) {
Object.assign(option, override);
}
});
return copy;
}
8 changes: 6 additions & 2 deletions tests/e2e/tests/test/e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
ng,
npm,
execAndWaitForOutputToMatch,
silentExecAndWaitForOutputToMatch,
killAllProcesses
} from '../../utils/process';
import { updateJsonFile } from '../../utils/project';
Expand Down Expand Up @@ -40,6 +41,9 @@ export default function () {
.then(() => killAllProcesses(), (err: any) => {
killAllProcesses();
throw err;
});

})
// Should run side-by-side with `ng serve`
.then(() => silentExecAndWaitForOutputToMatch('ng', ['serve'],
/webpack: Compiled successfully./))
.then(() => ng('e2e'));
}