Skip to content

Commit

Permalink
CI Log msg only if PERCY_CLIENT_ERROR_LOGS not set (#1576)
Browse files Browse the repository at this point in the history 
* CI Log msg only if PERCY_CLIENT_ERROR_LOGS not set

* FIxing specs
  • Loading branch information
@rishigupta1599
rishigupta1599 authored Apr 24, 2024
1 parent fe4d7e9 commit 174ff4c
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 91 deletions.
39 changes: 12 additions & 27 deletions packages/cli-exec/test/exec.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ describe('percy exec', () => {
let { default: which } = await import('which');
spyOn(which, 'sync').and.callFake(c => c);
spyOn(process, 'exit').and.callFake(c => c);
process.env.PERCY_CLIENT_ERROR_LOGS = false;
});

afterEach(() => {
Expand Down Expand Up @@ -74,14 +75,12 @@ describe('percy exec', () => {
it('starts and stops the percy process around the command', async () => {
await exec(['--', 'node', '--eval', '']);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual([
'[percy] Percy has started!',
'[percy] Running "node --eval "',
'[percy] Finalized build #1: https://percy.io/test/test/123',
'[percy] Build logs sent successfully. Please share this log ID with Percy team in case of any issues - random_sha'
"[percy] Build's CLI logs sent successfully. Please share this log ID with Percy team in case of any issues - random_sha"
]);
});

Expand Down Expand Up @@ -127,9 +126,7 @@ describe('percy exec', () => {
exec(['--', 'node', '--eval', 'process.exit(3)'])
).toBeRejectedWithError('EEXIT: 3');

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Running "node --eval process.exit(3)"',
Expand All @@ -142,9 +139,7 @@ describe('percy exec', () => {
await exec(['--', 'echo', 'Hi!']);

expect(stdoutSpy).toHaveBeenCalled();
expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Running "echo Hi!"',
Expand All @@ -153,13 +148,14 @@ describe('percy exec', () => {
});

it('adds process.stderr logs in CI logs', async () => {
process.env.PERCY_CLIENT_ERROR_LOGS = true;
let stderrSpy = spyOn(process.stderr, 'write').and.resolveTo(jasmine.stringMatching(/Some error/));
await expectAsync(
exec(['--', 'node', './test/test-data/test_prog.js', 'error']) // Throws Error
).toBeRejectedWithError('EEXIT: 1');

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
'[percy] Notice: Percy collects CI logs for service improvement, stored for 30 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
Expand Down Expand Up @@ -206,9 +202,7 @@ describe('percy exec', () => {
// user termination is not considered an error
await expectAsync(test).toBeResolved();

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).not.toContain(
'[percy] Running "node --eval "');
});
Expand All @@ -223,7 +217,6 @@ describe('percy exec', () => {
await expectAsync(exec(['--', 'foobar'])).toBeRejected();
expect(stdinSpy).toHaveBeenCalled();
expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false',
'[percy] Error: spawn error'
]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
Expand Down Expand Up @@ -251,9 +244,7 @@ describe('percy exec', () => {
// user termination is not considered an error
await expectAsync(test).toBeResolved();

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toContain(
'[percy] Stopping percy...'
);
Expand All @@ -268,9 +259,7 @@ describe('percy exec', () => {
'await request(url).catch(e => (console.error(e), process.exit(2)));'
].join('')]);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
jasmine.stringMatching('\\[percy] Running "node '),
Expand All @@ -283,9 +272,7 @@ describe('percy exec', () => {
'process.env.PERCY_BUILD_ID === "123" || process.exit(2)'
)]);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
jasmine.stringMatching('\\[percy] Running "node '),
Expand All @@ -298,9 +285,7 @@ describe('percy exec', () => {
'process.env.PERCY_BUILD_URL === "https://percy.io/test/test/123" || process.exit(2)'
)]);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
jasmine.stringMatching('\\[percy] Running "node '),
Expand Down
10 changes: 4 additions & 6 deletions packages/cli-snapshot/test/directory.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ describe('percy snapshot <directory>', () => {
'test-index/index.html': '<p>Test Index</p>'
}
});
process.env.PERCY_CLIENT_ERROR_LOGS = false;
});

afterEach(() => {
delete process.env.PERCY_TOKEN;
delete process.env.PERCY_ENABLE;
delete snapshot.packageInformation;
delete process.env.PERCY_CLIENT_ERROR_LOGS;
});

it('errors when the base-url is invalid', async () => {
Expand All @@ -39,9 +41,7 @@ describe('percy snapshot <directory>', () => {
it('starts a static server and snapshots matching files', async () => {
await snapshot(['./', '--include=/test-*.html']);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Snapshot taken: /test-1.html',
Expand All @@ -55,9 +55,7 @@ describe('percy snapshot <directory>', () => {
it('snapshots matching files hosted with a base-url', async () => {
await snapshot(['./', '--base-url=/base']);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Snapshot taken: /base/test-1.html',
Expand Down
32 changes: 10 additions & 22 deletions packages/cli-snapshot/test/file.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,12 @@ describe('percy snapshot <file>', () => {
].join('\n')
}
});
process.env.PERCY_CLIENT_ERROR_LOGS = false;
});

afterEach(async () => {
delete process.env.PERCY_TOKEN;
delete process.env.PERCY_CLIENT_ERROR_LOGS;
delete snapshot.packageInformation;
await server.close();
});
Expand All @@ -64,7 +66,7 @@ describe('percy snapshot <file>', () => {
await expectAsync(snapshot(['./nope'])).toBeRejected();

expect(logger.stdout).toEqual([
'[percy] Build logs sent successfully. Please share this log ID with Percy team in case of any issues - random_sha'
"[percy] Build's CLI logs sent successfully. Please share this log ID with Percy team in case of any issues - random_sha"
]);
expect(logger.stderr).toEqual(jasmine.arrayContaining([
'[percy] Error: Unsupported filetype: ./nope'
Expand All @@ -87,9 +89,7 @@ describe('percy snapshot <file>', () => {
it('snapshots pages from .yaml files', async () => {
await snapshot(['./pages.yml']);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Snapshot taken: YAML Snapshot',
Expand All @@ -106,9 +106,7 @@ describe('percy snapshot <file>', () => {

await snapshot(['./pages.json']);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Snapshot taken: JSON Snapshot',
Expand All @@ -120,9 +118,7 @@ describe('percy snapshot <file>', () => {
it('snapshots pages from .js files', async () => {
await snapshot(['./pages.js']);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Snapshot taken: JS Snapshot',
Expand All @@ -136,9 +132,7 @@ describe('percy snapshot <file>', () => {
it('snapshots pages from .js files that export a function', async () => {
await snapshot(['./pages-fn.cjs']); // .(c|m)?js

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Snapshot taken: JS Function Snapshot',
Expand All @@ -150,9 +144,7 @@ describe('percy snapshot <file>', () => {
it('snapshots pages from a list of URLs', async () => {
await snapshot(['./urls.yml', '--base-url=http://localhost:8000']);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Snapshot taken: /',
Expand All @@ -175,9 +167,7 @@ describe('percy snapshot <file>', () => {

await snapshot(['./lengthy.js', '--include=*2', '--exclude=/[13579]/']);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Snapshot taken: Snapshot #2',
Expand Down Expand Up @@ -250,9 +240,7 @@ describe('percy snapshot <file>', () => {

await snapshot(['./references.yaml']);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Snapshot taken: Reference Snapshot',
Expand Down
20 changes: 6 additions & 14 deletions packages/cli-upload/test/upload.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('percy upload', () => {
beforeEach(async () => {
upload.packageInformation = { name: '@percy/cli-upload' };
process.env.PERCY_TOKEN = 'web_<<PERCY_TOKEN>>';

process.env.PERCY_CLIENT_ERROR_LOGS = false;
await setupTest({
filesystem: {
'images/test-1.png': pixel,
Expand All @@ -26,6 +26,7 @@ describe('percy upload', () => {
afterEach(() => {
delete process.env.PERCY_TOKEN;
delete process.env.PERCY_ENABLE;
delete process.env.PERCY_CLIENT_ERROR_LOGS;
delete upload.packageInformation;
});

Expand Down Expand Up @@ -69,9 +70,7 @@ describe('percy upload', () => {
it('creates a new build and uploads snapshots with web token', async () => {
await upload(['./images']);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Uploading 3 snapshots...',
Expand Down Expand Up @@ -126,9 +125,7 @@ describe('percy upload', () => {
it('strips file extensions with `--strip-extensions`', async () => {
await upload(['./images', '--strip-extensions']);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Uploading 3 snapshots...',
Expand All @@ -142,9 +139,7 @@ describe('percy upload', () => {
it('skips unsupported image types', async () => {
await upload(['./images', '--files=*']);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Skipping unsupported file type: test-4.gif',
Expand Down Expand Up @@ -203,7 +198,6 @@ describe('percy upload', () => {
await up;

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false',
'[percy] AbortError: SIGTERM'
]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
Expand All @@ -219,9 +213,7 @@ describe('percy upload', () => {
process.env.PERCY_TOKEN = 'ss_<<PERCY_TOKEN>>';
await upload(['./images']);

expect(logger.stderr).toEqual([
'[percy] Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false'
]);
expect(logger.stderr).toEqual([]);
expect(logger.stdout).toEqual(jasmine.arrayContaining([
'[percy] Percy has started!',
'[percy] Uploading 3 snapshots...',
Expand Down
9 changes: 6 additions & 3 deletions packages/core/src/percy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,9 @@ export class Percy {
this.readyState = 0;

try {
this.log.warn('Notice: Percy collects CI logs for service improvement, stored for 14 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false');
if (process.env.PERCY_CLIENT_ERROR_LOGS !== 'false') {
this.log.warn('Notice: Percy collects CI logs for service improvement, stored for 30 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false');
}
// start the snapshots queue immediately when not delayed or deferred
if (!this.delayUploads && !this.deferUploads) yield this.#snapshots.start();
// do not start the discovery queue when not needed
Expand Down Expand Up @@ -424,7 +426,8 @@ export class Percy {
clilogs: logger.query(() => true)
};
// Only add CI logs if not disabled voluntarily.
if (process.env.PERCY_CLIENT_ERROR_LOGS !== 'false') {
const sendCILogs = process.env.PERCY_CLIENT_ERROR_LOGS !== 'false';
if (sendCILogs) {
const redactedContent = redactSecrets(logger.query(() => true, true));
logsObject.cilogs = redactedContent;
}
Expand All @@ -439,7 +442,7 @@ export class Percy {
};
// Ignore this will update once I implement logs controller.
const logsSHA = await this.client.sendBuildLogs(eventObject);
this.log.info(`Build logs sent successfully. Please share this log ID with Percy team in case of any issues - ${logsSHA}`);
this.log.info(`Build's CLI${sendCILogs ? ' and CI' : ''} logs sent successfully. Please share this log ID with Percy team in case of any issues - ${logsSHA}`);
} catch (err) {
this.log.warn('Could not send the builds logs');
}
Expand Down
Loading

0 comments on commit 174ff4c

Please sign in to comment.