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.

Sign up for GitHub

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

Jump to bottom

FLUID-5799, FLUID-5967: Reducing Fragility #5

Merged
merged 23 commits into from
Jan 24, 2017
Merged

FLUID-5799, FLUID-5967: Reducing Fragility #5

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
c863229
FLUID-5799: Check existence of remote repo
jobara Nov 27, 2015
d8bf7b2
FLUID-5799: Refactored to log all exec commands
jobara Nov 28, 2015
a64ac5f
FLUID-5799: Providing hints when errors occur.
jobara Aug 11, 2016
4a37de5
NOJIRA: Updated to ESLint and linted code
jobara Aug 11, 2016
2edf5c8
FLUID-5799: Version bump
jobara Aug 11, 2016
bf764ca
NOJIRA: Updated fluid-grunt-eslint to fix count of linted files
jobara Aug 24, 2016
140c2cd
FLUID-5799: Bumping node requirement to 4.0.0
jobara Aug 30, 2016
0a69275
NOJIRA: Updating Linting rules
jobara Aug 31, 2016
be30428
FLUID-5967: added an option to get the module's version
jobara Sep 22, 2016
49e5978
NOJIRA: Updating grunt dependency
jobara Sep 27, 2016
36feebf
NOJIRA: Updated to sharable eslint config
jobara Oct 3, 2016
cd241fc
FLUID-5799: Cleaning up README
jobara Oct 3, 2016
cb85d89
FLUID-5799: Command line options refactored
jobara Oct 3, 2016
099f773
FLUID-5799: Removed "\n" from log statement
jobara Dec 8, 2016
0e0d366
FLUID-5799: Fixed unit tests
jobara Dec 8, 2016
7a4f85a
FLUID-5799: Cleanup after updating version in dev builds
jobara Dec 8, 2016
74478a1
FLUID-5799: Refactoring getCLIOpts and updating related docs
jobara Dec 8, 2016
66d5a98
FLUID-5799: Fixing bug with tagging dev releases
jobara Dec 20, 2016
2775e6a
FLUID-5799: Updating hint descriptions
jobara Jan 12, 2017
3ed2231
FLUID-5799: Clarifying tag requirements for dev releases.
jobara Jan 12, 2017
e663e25
FLUID-5799: using publishDevHint for dev releases
jobara Jan 12, 2017
35a2d3c
FLUID-5799: Updated test comment
jobara Jan 12, 2017
f7da24a
FLUID-5799: More clarifications to the publishDevHint
jobara Jan 24, 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
35 changes: 10 additions & 25 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,45 +199,30 @@ publish.standard();
</tr>
<tr>
<td>
<code>versionCmd</code>
<code>publishDevCmd</code>
</td>
<td>
The CLI to execute which sets the dev version to release under.
<ul>
<li>
<code>${version}</code> will be substituted with the generated dev build version.
</li>
</ul>
The CLI to execute which publishes a development release to NPM.
Uses the value specified by the `devTag` option.
</td>
<td>
"npm version --no-git-tag-version ${version}"
<br>
<br>
<p>
<em><strong>NOTE</strong>: This command will update the version in the package.json file, but will not commit the change.</em>
</p>
"npm publish --tag ${devTag}"
</td>
</tr>
<tr>
<td>
<code>distTagCmd</code>
<code>versionCmd</code>
</td>
<td>
The CLI to execute which tags an NPM release.
The CLI to execute which sets the dev version to release under.
<ul>
<li>
<code>${packageName}</code> will be substituted with executing module's name.
</li>
<li>
<code>${version}</code> will be substituted with the generated dev build version.
</li>
<li>
<code>${tag}</code>will be substituted with the value from the <code>devTag</code> option.
</li>
</ul>
</td>
<td>
"npm dist-tag add ${packageName}@${version} ${tag}"
"npm version --no-git-tag-version ${version}"
<br>
<br>
<p>
Expand Down Expand Up @@ -372,13 +357,13 @@ publish.standard();
</tr>
<tr>
<td>
<code>distTagHint</code>
<code>publishDevHint</code>
</td>
<td>
A hint for addressing an issue where applying a distribution tag fails.
A hint for addressing an issue where publishing to the registry fails.
Copy link
Member

Choose a reason for hiding this comment

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

Description should distinguish from publishHint

</td>
<td>
"If the tag already exists use a new tag name or run \"npm dist-tag rm ${packageName} ${tag}\" to remove the existing one.\nAlso ensure that the tag name is valid (i.e. doesn't conform to a valid semver range like v1.4 or 1.4).\n"
"Ensure that you have access to publish to the registry and that the current version does not already exist.\nIf the tag already exists use a new tag name or run \"npm dist-tag rm ${packageName} ${tag}\" to remove the existing one.\nAlso ensure that the tag name is valid (i.e. doesn't conform to a valid semver range like v1.4 or 1.4).\n"
Copy link
Member

Choose a reason for hiding this comment

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

The last sentence is puzzlingly worded - is it the case that a tag name CANNOT correspond to a semver version, or is it the opposite?

</td>
</tr>
<tr>
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@
"revisionCmd": "git rev-parse --verify --short HEAD",
"packCmd": "npm pack",
"publishCmd": "npm publish",
"publishDevCmd": "npm publish --tag ${devTag}",
"versionCmd": "npm version --no-git-tag-version ${version}",
"distTagCmd": "npm dist-tag add ${packageName}@${version} ${tag}",
"cleanCmd": "git checkout -- package.json",
"vcTagCmd": "git tag -a v${version} -m 'Tagging the ${version} release'",
"pushVCTagCmd": "git push ${remote} v${version}",
Expand All @@ -58,7 +58,7 @@
"changesHint": "Address uncommitted changes: Commit \"git commit -a\", Stash \"git stash\" or Clean \"git reset --hard\"\n",
"checkRemoteHint": "Run \"git remote -v\" for a list of available remote repositories.\n",
"publishHint": "Ensure that you have access to publish to the registry and that the current version does not already exist.\n",
"distTagHint": "If the tag already exists use a new tag name or run \"npm dist-tag rm ${packageName} ${tag}\" to remove the existing one.\nAlso ensure that the tag name is valid (i.e. doesn't conform to a valid semver range like v1.4 or 1.4).\n",
"publishDevHint": "Ensure that you have access to publish to the registry and that the current version does not already exist.\nIf the tag already exists use a new tag name or run \"npm dist-tag rm ${packageName} ${tag}\" to remove the existing one.\nAlso ensure that the tag name is valid (i.e. doesn't conform to a valid semver range like v1.4 or 1.4).\n",
"vcTagHint": "If the tag already exists, run \"git tag -d v${version}\" to remove the existing tag.\n",
"pushVCTagHint": "If the tag already exists, run \"git push ${remote} :refs/tags/v${version} to remove the existing tag.\n"
}
Expand Down
30 changes: 7 additions & 23 deletions publish.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,35 +223,20 @@ publish.getDevVersion = function (moduleVersion, options) {
* If isTest is specified, it will instead create a tarball in the local directory.
*
* @param isTest {Boolean} - indicates if this is a test run or not
* @param options {Object} - e.g. {"packCmd": "npm pack", "publishCmd": "npm publish", "publishHint": "publish hint"}
* @param isDev {Boolean} - indicates if this is a development (true) or standard (false) release
* @param options {Object} - e.g. {"packCmd": "npm pack", "publishCmd": "npm publish", "publishDevCmd": "npm publish --tag", "publishHint": "publish hint", "publishDevHint": "publish dev hint", devTag: "dev"}
*/
publish.pubImpl = function (isTest, options) {
publish.pubImpl = function (isTest, isDev, options) {
if (isTest) {
// create a local tarball
publish.execSyncFromTemplate(options.packCmd);
} else {
// publish to npm
publish.execSyncFromTemplate(options.publishCmd, {}, options.publishHint);
var pubCmd = isDev ? options.publishDevCmd : options.publishCmd;
publish.execSyncFromTemplate(pubCmd, options, options.publishHint);
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this command doesn't use options.publishDevHint when publishing a dev release, as I imagine it should

}
};

/**
* Tags the specified version with the specified dist-tag
* If it is a test run, the tag command will be output to the console.
*
* @param isTest {Boolean} - indicates if this is a test run or not
* @param version {String} - a string indicating which version to tag
* @param tag {String} - the dist-tag to apply
* @param options {Object} - e.g. {"distTagCmd": "npm dist-tag add ${packageName}@${version} ${tag}", "distTagHint": "dist tag hint"}
*/
publish.tag = function (isTest, packageName, version, tag, options) {
publish.execSyncFromTemplate(options.distTagCmd, {
packageName: packageName,
version: version,
tag: tag
}, options.distTagHint, isTest);
};

/**
* Applies a version control tag to the latest commit
*
Expand Down Expand Up @@ -328,8 +313,7 @@ publish.dev = function (isTest, options) {

try {
// publish
publish.pubImpl(isTest, opts);
publish.tag(isTest, modulePkg.name, devVersion, opts.devTag, opts);
publish.pubImpl(isTest, true, opts);
} finally {
// cleanup changes
publish.clean(opts.moduleRoot, opts);
Expand Down Expand Up @@ -365,7 +349,7 @@ publish.standard = function (isTest, options) {
publish.tagVC (isTest, modulePkg.version, opts);

// publish
publish.pubImpl(isTest, opts);
publish.pubImpl(isTest, false, opts);
};

module.exports = publish;
Expand Down
117 changes: 51 additions & 66 deletions tests/publishTests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,81 +318,68 @@ console.log("\n*** publish.pubImpl ***");

var pubImplFixture = [{
Copy link
Member

Choose a reason for hiding this comment

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

A small strategy hint comment would help the reader here - something like 'In the cases where commands should not execute, their entry in the structure has been filled with a placeholder string beginning "shouldn't"'

isTest: true,
isDev: false,
packCmd: "pack",
publishCmd: "shouldn't publish"
publishCmd: "shouldn't publish",
publishDevCmd: "shouldn't publish dev"
}, {
isTest: false,
isDev: false,
packCmd: "shouldn't pack",
publishCmd: "publish"
publishCmd: "publish",
publishDevCmd: "shouldn't publish dev"
}, {
isDev: false,
packCmd: "shouldn't pack",
publishCmd: "publish"
publishCmd: "publish",
publishDevCmd: "shouldn't publish dev"
}, {
isDev: true,
packCmd: "shouldn't pack",
publishCmd: "shouldn't publish",
publishDevCmd: "publish dev"
}, {
isTest: false,
isDev: true,
packCmd: "pack",
publishCmd: "shouldn't publish",
publishDevCmd: "publish dev"
}, {
isTest: true,
isDev: true,
packCmd: "pack",
publishCmd: "shouldn't publish",
publishDevCmd: "shouldn't publish dev"
}, {
isTest: false,
packCmd: "shouldn't pack",
publishCmd: "publish",
publishDevCmd: "shouldn't publish dev"
}, {
isTest: true,
packCmd: "pack",
publishCmd: "shouldn't publish",
publishDevCmd: "shouldn't publish dev"
}, {
packCmd: "shouldn't pack",
publishCmd: "publish",
publishDevCmd: "shouldn't publish dev"
}];

pubImplFixture.forEach(function (fixture) {
console.log("pubImpl test - isTest: " + fixture.isTest + " packCmd: " + fixture.packCmd + " publishCmd: " + fixture.publishCmd);
console.log("pubImpl test - isTest: " + fixture.isTest + "isDev: " + fixture.isDev + " packCmd: " + fixture.packCmd + " publishCmd: " + fixture.publishCmd + " publishDevCmd: " + fixture.publishDevCmd);

var exec = sinon.stub(publish, "execSync");
var expected = fixture[fixture.isTest ? "packCmd" : "publishCmd"];
var expected = fixture.isTest ? fixture.packCmd : fixture[fixture.isDev ? "publishDevCmd" : "publishCmd"];

publish.pubImpl(fixture.isTest, fixture);
publish.pubImpl(fixture.isTest, fixture.isDev, fixture);
assert(exec.calledOnce, "execSync should have been called");
assert(exec.calledWith(expected), "execSync should have been called with: " + expected);

// remove execSync stub
publish.execSync.restore();
});

/***************
* publish.tag *
***************/
console.log("\n*** publish.tag ***");

var tagFixture = [{
isTest: true,
packageName: "test1",
version: "1.0.0",
tag: "tag",
distTagCmd: "add tag ${tag} to ${packageName} at ${version}",
expected: "add tag tag to test1 at 1.0.0"
}, {
isTest: false,
packageName: "test2",
version: "2.0.0",
tag: "tag2",
distTagCmd: "add tag ${tag} to ${packageName} at ${version}",
expected: "add tag tag2 to test2 at 2.0.0"
}, {
version: "3.0.0",
packageName: "test3",
tag: "tag3",
distTagCmd: "add tag ${tag} to ${packageName} at ${version}",
expected: "add tag tag3 to test3 at 3.0.0"
}];

tagFixture.forEach(function (fixture) {
console.log("tag test - isTest: " + fixture.isTest + " packageName: " + fixture.packageName + " version: " + fixture.version + " tag: " + fixture.tag + " distTagCmd: " + fixture.distTagCmd);

var toStub = ["execSync", "log"];
var stub = createStubs(publish, toStub);
var expectedLog = "Executing Command: " + fixture.expected;

publish.tag(fixture.isTest, fixture.packageName, fixture.version, fixture.tag, fixture);

assert(stub.log.calledOnce, "console.log should have been called");
assert(stub.log.calledWith(expectedLog), "console.log should have been called with: " + expectedLog);

if (fixture.isTest) {
assert(!stub.execSync.called, "execSync should not have been called");
} else {
assert(stub.execSync.calledOnce, "execSync should have been called");
assert(stub.execSync.calledWith(fixture.expected), "execSync should have been called with: " + fixture.expected);
}

// remove stubs
removeStubs(publish, toStub);
});

/*****************
* publish.clean *
*****************/
Expand Down Expand Up @@ -461,8 +448,8 @@ var publishFixture = [{
"revisionCmd": "dry run get revision",
"packCmd": "dry run pack",
"publishCmd": "dry run publish",
"publishDevCmd": "dry run npm publish dev",
"versionCmd": "dry run version",
"distTagCmd": "dry run set tag",
"cleanCmd": "dry run clean",
"vcTagCmd": "dry run vc tag",
"pushVCTagCmd": "dry run push vc tag",
Expand All @@ -473,7 +460,7 @@ var publishFixture = [{
"changesHint": "dry run changes hint\n",
"checkRemoteHint": "dry run check remote hint\n",
"publishHint": "dry run publish hint\n",
"distTagHint": "dry run dist tag hint\n",
"publishDevHint": "dry run publish dev hint\n",
"vcTagHint": "dry run vc tag hint\n",
"pushVCTagHint": "dry run push vc tag hint\n"
}
Expand All @@ -486,8 +473,8 @@ var publishFixture = [{
"revisionCmd": "get revision",
"packCmd": "pack",
"publishCmd": "publish",
"publishDevCmd": "publish dev",
"versionCmd": "version",
"distTagCmd": "set tag",
"cleanCmd": "clean",
"vcTagCmd": "vc tag",
"pushVCTagCmd": "push vc tag",
Expand All @@ -498,7 +485,7 @@ var publishFixture = [{
"changesHint": "changes hint\n",
"checkRemoteHint": "check remote hint\n",
"publishHint": "publish hint\n",
"distTagHint": "dist tag hint\n",
"publishDevHint": "publish dev hint\n",
"vcTagHint": "vc tag hint\n",
"pushVCTagHint": "push vc tag hint\n"
}
Expand All @@ -515,7 +502,7 @@ publishFixture.forEach(function (fixture) {
var optsString = JSON.stringify(fixture.options || {});
console.log("dev test - isTest: " + fixture.isTest, " options: " + optsString + "\n");

var toStub = ["checkChanges", "getDevVersion", "setVersion", "pubImpl", "tag", "clean"];
var toStub = ["checkChanges", "getDevVersion", "setVersion", "pubImpl", "clean"];
var stub = createStubs(publish, toStub);
var moduleVersion = modulePackage.version;
var devVersion = moduleVersion + "-testVersion";
Expand All @@ -530,11 +517,9 @@ publishFixture.forEach(function (fixture) {
assert(stub.setVersion.calledOnce, "setVersion should have been called");
assert(stub.setVersion.calledWith(devVersion, fixture.options), "setVersion should have been called with: " + devVersion + ", " + optsString);
assert(stub.pubImpl.calledOnce, "pubImpl should have been called");
assert(stub.pubImpl.calledWith(fixture.isTest, fixture.options), modulePackage);
assert(stub.tag.calledOnce, "tag should have been called");
assert(stub.tag.calledWith(fixture.isTest, modulePackage.name, devVersion, fixture.options.devTag, fixture.options), "tag should have been called with: " + fixture.isTest + ", " + modulePackage.name + ", " + devVersion + ", " + fixture.options.devTag + ", " + optsString);
assert(stub.pubImpl.calledWith(fixture.isTest, true, fixture.options), modulePackage);
assert(stub.clean.calledOnce, "clean should have been called");
assert(stub.clean.calledWith(fixture.options.moduleRoot, fixture.options), "clean should have been called with: " + fixture.options.moduleRoot + ", " + optsString);
assert(stub.clean.calledWith(fixture.options.moduleRoot, fixture.options), "clean should have been called with: " + fixture.options.moduleRoot + ", true, " + optsString);

removeStubs(publish, toStub);
});
Expand All @@ -561,7 +546,7 @@ publishFixture.forEach(function (fixture) {
assert(stub.tagVC.calledOnce, "tagVC should have been called");
assert(stub.tagVC.calledWith(fixture.isTest, modulePackage.version, fixture.options), "tagVC should have been called with: " + fixture.isTest + " ," + modulePackage.version + " ," + optsString);
assert(stub.pubImpl.calledOnce, "pubImpl should have been called");
assert(stub.pubImpl.calledWith(fixture.isTest, fixture.options), "pubImpl should have been called with: " + fixture.isTest + ", " + optsString);
assert(stub.pubImpl.calledWith(fixture.isTest, false, fixture.options), "pubImpl should have been called with: " + fixture.isTest + ", false, " + optsString);

removeStubs(publish, toStub);
});