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

[APM] Script static checkers + precommit #77900

Merged
merged 4 commits into from
Sep 21, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ export async function aggregateLatencyMetrics() {
return;
}

const response = await destClient?.bulk({
const response = await (destClient as any)?.bulk({
refresh: 'wait_for',
body: flatten(
docs.map((doc) => [
Expand Down
37 changes: 37 additions & 0 deletions x-pack/plugins/apm/scripts/eslint.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
//eslint-disable-next-line import/no-extraneous-dependencies
const { CLIEngine } = require('eslint');
const { resolve } = require('path');
//eslint-disable-next-line import/no-extraneous-dependencies
const { argv } = require('yargs');

async function run() {
const fix = !!argv.fix;

const engine = new CLIEngine({
fix,
cache: true,
extensions: ['.js', '.jsx', '.ts', '.tsx'],
});
Copy link
Member

Choose a reason for hiding this comment

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

Does this automatically find the right eslintrc?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, looks like it.


const report = engine.executeOnFiles(resolve(__dirname, '..'));

const formatter = engine.getFormatter();

return formatter(report.results);
}

run()
.then((text) => {
//eslint-disable-next-line no-console
console.log(text);
process.exit(0);
})
.catch((err) => {
console.error(err);
process.exit(1);
});
25 changes: 25 additions & 0 deletions x-pack/plugins/apm/scripts/jest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
// eslint-disable-next-line import/no-extraneous-dependencies
require('@babel/register')({
extensions: ['.js'],
plugins: [],
presets: [
'@babel/typescript',
['@babel/preset-env', { targets: { node: 'current' } }],
],
});

// eslint-disable-next-line import/no-extraneous-dependencies
const { run } = require('jest');

process.env.NODE_ENV = process.env.NODE_ENV || 'test';

const config = require('../jest.config.js');

const argv = [...process.argv.slice(2), '--config', JSON.stringify(config)];
Copy link
Member

@sorenlouv sorenlouv Sep 21, 2020

Choose a reason for hiding this comment

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

Looks like this could/should be a path
image

Suggested change
const argv = [...process.argv.slice(2), '--config', JSON.stringify(config)];
const argv = [...process.argv.slice(2), '--config', '../jest.config.js'];

(might have to do something with path.resolve('..', 'jest.config.js') to get the correct path)

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the same approach as the top-level jest test runner uses.


run(argv);
69 changes: 69 additions & 0 deletions x-pack/plugins/apm/scripts/precommit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

/* eslint-disable no-console*/
/* eslint-disable import/no-extraneous-dependencies*/

const execa = require('execa');
const Listr = require('listr');
const { resolve } = require('path');

const cwd = resolve(__dirname, '../../../..');

const execaOpts = { cwd, stderr: 'pipe' };

const tasks = new Listr(
[
{
title: 'Jest',
task: () =>
execa(
'node',
[
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we just do require.resolve('jest/bin/jest') here? Not sure what the jest script written here does other than run the CLI.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is that the same as running yarn jest --config x-pack/plugins/apm/jest.config.js? Because looks like the latter is a little slower to pick up files.

resolve(__dirname, './jest.js'),
'--reporters',
resolve(__dirname, './node_modules/jest-silent-reporter'),
'--collect-coverage',
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is just for precommit we might want to turn off coverage

Copy link
Member Author

Choose a reason for hiding this comment

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

done

'false',
],
execaOpts
),
},
{
title: 'Typescript',
task: () =>
execa('node', [resolve(__dirname, 'optimize-tsconfig.js')]).then(() =>
execa(
require.resolve('typescript/bin/tsc'),
[
'--project',
resolve(__dirname, '../../../tsconfig.json'),
'--pretty',
'--noEmit',
'--skipLibCheck',
],
execaOpts
)
),
},
{
title: 'Lint',
task: () => execa('node', [resolve(__dirname, 'eslint.js')], execaOpts),
},
],
{ exitOnError: false, concurrent: true }
);

tasks.run().catch((error) => {
// from src/dev/typescript/exec_in_projects.ts
process.exitCode = 1;

const errors = error.errors || [error];

for (const e of errors) {
process.stderr.write(e.stdout);
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export async function createOrUpdateIndex({
await client.indices.exists({
index: indexName,
})
).body as boolean;
).body as unknown;

if (!indexExists) {
await client.indices.create({
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/apm/scripts/upload-telemetry-data/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ async function uploadData() {
apmAgentConfigurationIndex: '.apm-agent-configuration',
},
search: (body) => {
return client.search(body as any).then((res) => res.body);
return client.search(body as any).then((res) => res.body as any);
},
indicesStats: (body) => {
return client.indices.stats(body as any).then((res) => res.body);
Expand Down