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

fix the location for yarn-error.log and add a test case #2870

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
41 changes: 41 additions & 0 deletions __tests__/cli/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/* @flow */
const path = require('path');
const exec = require('child_process').exec;
import * as fs from '../../src/util/fs.js';
import makeTemp from '../_temp.js';
import NoopReporter from '../../src/reporters/base-reporter.js';
import * as constants from '../../src/constants.js';
import assert from 'assert';

const yarnBin = path.join(__dirname, '..', '..', 'bin', 'yarn.js');
const fixturesLoc = path.join(__dirname, '..', 'fixtures', 'index');

async function setupWorkingDir(fixture: string): Promise<string> {
const srcDir = path.join(fixturesLoc, fixture);
const workingDir = await makeTemp(fixture);
await fs.copy(srcDir, workingDir, new NoopReporter());

return workingDir;
}

function execCommand(workingDir: string, cacheDir: string): Promise<string> {
return new Promise((resolve, reject) => {
exec(`node "${yarnBin}" tag rm non-existing-pkg non-existing-tag --cache-folder ${cacheDir} | cat`,
{cwd: workingDir}, (err, stdout) => {
if (err) {
reject(err);
} else {
resolve(workingDir);
}
});
});
}

test('Verify path errorReport log', async () => {
const workingDir = await setupWorkingDir('run-failing-custom-script');
const cacheDir = path.join(workingDir, 'cache');
await fs.mkdirp(cacheDir);

await execCommand(workingDir, cacheDir);
assert.ok(await fs.exists(path.join(cacheDir, 'v' + String(constants.CACHE_VERSION), 'yarn-error.log')));
});
11 changes: 11 additions & 0 deletions __tests__/fixtures/index/run-failing-custom-script/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "test_run_failing_custom_script",
"version": "1.0.0",
"license": "UNLICENSED",
"scripts": {
"custom-script": "tag rm non-existing-pkg non-existing-tag"
},
"dependencies": {
"n": "^2.1.4"
}
}
4 changes: 2 additions & 2 deletions src/cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ function onUnexpectedError(err: Error) {
}

function writeErrorReport(log) : ?string {
const errorReportLoc = path.join(config.cwd, 'yarn-error.log');
const errorReportLoc = path.join(config.cacheFolder, 'yarn-error.log');

try {
fs.writeFileSync(errorReportLoc, log.join('\n\n') + '\n');
Expand All @@ -362,7 +362,7 @@ config.init({
binLinks: commander.binLinks,
modulesFolder: commander.modulesFolder,
globalFolder: commander.globalFolder,
cacheRootFolder: commander.cacheFolder,
cacheFolder: commander.cacheFolder,
Copy link
Member

Choose a reason for hiding this comment

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

This is done on purpose afaik.
@arcanis we probably need to add a comment here.

Copy link
Author

Choose a reason for hiding this comment

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

This probably does need a change to either _cacheRootFolder or cacheFolder since cacheRootFolder does not exist in the configOptions export type. The reason for this change was the cache folder location not changing when setting it with the --cache-folder command (see #2881).

Copy link
Member

@arcanis arcanis Mar 10, 2017

Choose a reason for hiding this comment

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

Yep the specific line where this is used is here: https://github.com/yarnpkg/yarn/blob/master/src/config.js#L241

In order to prevent BC break we decided to keep using cacheFolder in the public interface instead of cacheRootFolder, but it seems I forgot one. Good catch @clanghout!

preferOffline: commander.preferOffline,
captureHar: commander.har,
ignorePlatform: commander.ignorePlatform,
Expand Down