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

[No QA][TS migration] Migrate remaining GitHub actions to TypeScript #38576

Merged
merged 11 commits into from
Mar 27, 2024
Merged

[No QA][TS migration] Migrate remaining GitHub actions to TypeScript #38576

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b11482b
migrate getGraphiteString and getArtifactInfo to TypeScript
JKobrynski Mar 19, 2024
a7dba17
update file extension in buildActions.sh
JKobrynski Mar 19, 2024
7fd4d91
apply suggested changes
JKobrynski Mar 19, 2024
394f9df
add todo comment
JKobrynski Mar 19, 2024
31e8c49
Merge branch 'main' into migrateGroup30ToTypeScript
JKobrynski Mar 20, 2024
16e5c0c
update type casting in getArtifactInfo
JKobrynski Mar 20, 2024
7648e0a
Merge branch 'main' into migrateGroup30ToTypeScript
JKobrynski Mar 26, 2024
97fa360
fix mocks import in test files
JKobrynski Mar 26, 2024
8d36a59
Merge branch 'main' into migrateGroup30ToTypeScript
JKobrynski Mar 26, 2024
3d32646
Merge branch 'main' into migrateGroup30ToTypeScript
JKobrynski Mar 27, 2024
81e823f
remove type castings related to GitHubUtils
JKobrynski Mar 27, 2024
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
23 changes: 13 additions & 10 deletions ...script/getArtifactInfo/getArtifactInfo.js → ...script/getArtifactInfo/getArtifactInfo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
const _ = require('underscore');
const core = require('@actions/core');
const GithubUtils = require('../../../libs/GithubUtils');
import * as core from '@actions/core';
import type {components as OctokitComponents} from '@octokit/openapi-types/types';
import GithubUtils from '../../../libs/GithubUtils';

const run = function () {
type OctokitArtifact = OctokitComponents['schemas']['artifact'];

const run = function (): Promise<OctokitArtifact | void> {
const artifactName = core.getInput('ARTIFACT_NAME', {required: true});
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const run = function (): Promise<OctokitArtifact | void> {
const run = function (): Promise<void> {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!


// TODO: remove type casting once GithubUtils (https://github.com/Expensify/App/pull/38280) is migrated to TypeScript
return GithubUtils.getArtifactByName(artifactName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we define our own typings for GithubUtils in the meanwhile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you mean, could you provide more details? 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you familiar with ambient typings and .d.ts files?

In general, typings can be provided by any party. Either the original third-party library itself (let's say foo), another third-party library (like @types/foo), or the project that depends on foo itself.

I'm not sure if there's any difficulty with using ambient typings with GitHub actions specifically, I only used them in "typical" TypeScript projects, but I would assume it's possible.

We can give up if this becomes too time-consuming, but I'd give it a try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I did it for some other modules, great idea! Actually GithubUtils PR got merged a couple hours ago so the problem is not valid anymore!

.then((artifact) => {
if (_.isUndefined(artifact)) {
.then((artifact: OctokitArtifact) => {
if (artifact === undefined) {
console.log(`No artifact found with the name ${artifactName}`);
core.setOutput('ARTIFACT_FOUND', false);
return;
Expand All @@ -16,16 +19,16 @@ const run = function () {
console.log('Artifact info', artifact);
core.setOutput('ARTIFACT_FOUND', true);
core.setOutput('ARTIFACT_ID', artifact.id);
core.setOutput('ARTIFACT_WORKFLOW_ID', artifact.workflow_run.id);
core.setOutput('ARTIFACT_WORKFLOW_ID', artifact.workflow_run?.id);
})
.catch((error) => {
.catch((error: Error) => {
console.error('A problem occurred while trying to communicate with the GitHub API', error);
core.setFailed(error);
});
}) as Promise<OctokitArtifact | void>;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}) as Promise<OctokitArtifact | void>;
}) as Promise<void>;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


if (require.main === module) {
run();
}

module.exports = run;
export default run;
102 changes: 63 additions & 39 deletions .github/actions/javascript/getArtifactInfo/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,44 +4,6 @@
/******/ (() => { // webpackBootstrap
/******/ var __webpack_modules__ = ({

/***/ 1307:
/***/ ((module, __unused_webpack_exports, __nccwpck_require__) => {

const _ = __nccwpck_require__(5067);
const core = __nccwpck_require__(2186);
const GithubUtils = __nccwpck_require__(7999);

const run = function () {
const artifactName = core.getInput('ARTIFACT_NAME', {required: true});

return GithubUtils.getArtifactByName(artifactName)
.then((artifact) => {
if (_.isUndefined(artifact)) {
console.log(`No artifact found with the name ${artifactName}`);
core.setOutput('ARTIFACT_FOUND', false);
return;
}

console.log('Artifact info', artifact);
core.setOutput('ARTIFACT_FOUND', true);
core.setOutput('ARTIFACT_ID', artifact.id);
core.setOutput('ARTIFACT_WORKFLOW_ID', artifact.workflow_run.id);
})
.catch((error) => {
console.error('A problem occurred while trying to communicate with the GitHub API', error);
core.setFailed(error);
});
};

if (require.main === require.cache[eval('__filename')]) {
run();
}

module.exports = run;


/***/ }),

/***/ 4097:
/***/ ((module) => {

Expand Down Expand Up @@ -13637,6 +13599,68 @@ function wrappy (fn, cb) {
}


/***/ }),

/***/ 7750:
/***/ (function(__unused_webpack_module, exports, __nccwpck_require__) {

"use strict";

var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
if (k2 === undefined) k2 = k;
var desc = Object.getOwnPropertyDescriptor(m, k);
if (!desc || ("get" in desc ? !m.__esModule : desc.writable || desc.configurable)) {
desc = { enumerable: true, get: function() { return m[k]; } };
}
Object.defineProperty(o, k2, desc);
}) : (function(o, m, k, k2) {
if (k2 === undefined) k2 = k;
o[k2] = m[k];
}));
var __setModuleDefault = (this && this.__setModuleDefault) || (Object.create ? (function(o, v) {
Object.defineProperty(o, "default", { enumerable: true, value: v });
}) : function(o, v) {
o["default"] = v;
});
var __importStar = (this && this.__importStar) || function (mod) {
if (mod && mod.__esModule) return mod;
var result = {};
if (mod != null) for (var k in mod) if (k !== "default" && Object.prototype.hasOwnProperty.call(mod, k)) __createBinding(result, mod, k);
__setModuleDefault(result, mod);
return result;
};
var __importDefault = (this && this.__importDefault) || function (mod) {
return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", ({ value: true }));
const core = __importStar(__nccwpck_require__(2186));
const GithubUtils_1 = __importDefault(__nccwpck_require__(7999));
const run = function () {
const artifactName = core.getInput('ARTIFACT_NAME', { required: true });
// TODO: remove type casting once GithubUtils (https://github.com/Expensify/App/pull/38280) is migrated to TypeScript
return GithubUtils_1.default.getArtifactByName(artifactName)
.then((artifact) => {
if (artifact === undefined) {
console.log(`No artifact found with the name ${artifactName}`);
core.setOutput('ARTIFACT_FOUND', false);
return;
}
console.log('Artifact info', artifact);
core.setOutput('ARTIFACT_FOUND', true);
core.setOutput('ARTIFACT_ID', artifact.id);
core.setOutput('ARTIFACT_WORKFLOW_ID', artifact.workflow_run?.id);
})
.catch((error) => {
console.error('A problem occurred while trying to communicate with the GitHub API', error);
core.setFailed(error);
});
};
if (require.main === require.cache[eval('__filename')]) {
run();
}
exports["default"] = run;


/***/ }),

/***/ 2877:
Expand Down Expand Up @@ -16000,7 +16024,7 @@ module.exports = JSON.parse('[[[0,44],"disallowed_STD3_valid"],[[45,46],"valid"]
/******/ // startup
/******/ // Load entry module and return exports
/******/ // This entry module is referenced by other modules so it can't be inlined
/******/ var __webpack_exports__ = __nccwpck_require__(1307);
/******/ var __webpack_exports__ = __nccwpck_require__(7750);
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for my ignorance. Can you explain to me why this changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is auto-generated so I don't know the exact reason behind that change, but my guess is this might be due to module.exports changing to export default. Wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh I see.

/******/ module.exports = __webpack_exports__;
/******/
/******/ })()
Expand Down
16 changes: 9 additions & 7 deletions ...pt/getGraphiteString/getGraphiteString.js → ...pt/getGraphiteString/getGraphiteString.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const core = require('@actions/core');
const github = require('@actions/github');
const fs = require('fs');
import * as core from '@actions/core';
import * as github from '@actions/github';
import fs from 'fs';

const run = () => {
// Prefix path to the graphite metric
Expand All @@ -11,8 +11,10 @@ const run = () => {
regressionOutput = JSON.parse(fs.readFileSync('.reassure/output.json', 'utf8'));
} catch (err) {
// Handle errors that occur during file reading or parsing
console.error('Error while parsing output.json:', err.message);
core.setFailed(err);
if (err instanceof Error) {
console.error('Error while parsing output.json:', err.message);
core.setFailed(err);
}
}

const creationDate = regressionOutput.metadata.current.creationDate;
Expand All @@ -22,7 +24,7 @@ const run = () => {
const timestamp = Math.floor(timestampInMili / 1000);

// Get PR number from the github context
const prNumber = github.context.payload.pull_request.number;
const prNumber = github.context.payload.pull_request?.number;

// We need to combine all tests from the 4 buckets
const reassureTests = [...regressionOutput.meaningless, ...regressionOutput.significant, ...regressionOutput.countChanged, ...regressionOutput.added];
Expand Down Expand Up @@ -51,4 +53,4 @@ if (require.main === module) {
run();
}

module.exports = run;
export default run;
144 changes: 82 additions & 62 deletions .github/actions/javascript/getGraphiteString/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,67 +4,6 @@
/******/ (() => { // webpackBootstrap
/******/ var __webpack_modules__ = ({

/***/ 1302:
/***/ ((module, __unused_webpack_exports, __nccwpck_require__) => {

const core = __nccwpck_require__(2186);
const github = __nccwpck_require__(5438);
const fs = __nccwpck_require__(7147);

const run = () => {
// Prefix path to the graphite metric
const GRAPHITE_PATH = 'reassure';

let regressionOutput;
try {
regressionOutput = JSON.parse(fs.readFileSync('.reassure/output.json', 'utf8'));
} catch (err) {
// Handle errors that occur during file reading or parsing
console.error('Error while parsing output.json:', err.message);
core.setFailed(err);
}

const creationDate = regressionOutput.metadata.current.creationDate;
const timestampInMili = new Date(creationDate).getTime();

// Graphite accepts timestamp in seconds
const timestamp = Math.floor(timestampInMili / 1000);

// Get PR number from the github context
const prNumber = github.context.payload.pull_request.number;

// We need to combine all tests from the 4 buckets
const reassureTests = [...regressionOutput.meaningless, ...regressionOutput.significant, ...regressionOutput.countChanged, ...regressionOutput.added];

// Map through every test and create string for meanDuration and meanCount
// eslint-disable-next-line rulesdir/prefer-underscore-method
const graphiteString = reassureTests
.map((test) => {
const current = test.current;
// Graphite doesn't accept metrics name with space, we replace spaces with "-"
const formattedName = current.name.split(' ').join('-');

const renderDurationString = `${GRAPHITE_PATH}.${formattedName}.renderDuration ${current.meanDuration} ${timestamp}`;
const renderCountString = `${GRAPHITE_PATH}.${formattedName}.renderCount ${current.meanCount} ${timestamp}`;
const renderPRNumberString = `${GRAPHITE_PATH}.${formattedName}.prNumber ${prNumber} ${timestamp}`;

return `${renderDurationString}\n${renderCountString}\n${renderPRNumberString}`;
})
.join('\n');

// Set generated graphite string to the github variable
core.setOutput('GRAPHITE_STRING', graphiteString);
};

if (require.main === require.cache[eval('__filename')]) {
run();
}

module.exports = run;


/***/ }),

/***/ 7351:
/***/ (function(__unused_webpack_module, exports, __nccwpck_require__) {

Expand Down Expand Up @@ -9565,6 +9504,87 @@ function wrappy (fn, cb) {
}


/***/ }),

/***/ 7717:
/***/ (function(__unused_webpack_module, exports, __nccwpck_require__) {

"use strict";

var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
if (k2 === undefined) k2 = k;
var desc = Object.getOwnPropertyDescriptor(m, k);
if (!desc || ("get" in desc ? !m.__esModule : desc.writable || desc.configurable)) {
desc = { enumerable: true, get: function() { return m[k]; } };
}
Object.defineProperty(o, k2, desc);
}) : (function(o, m, k, k2) {
if (k2 === undefined) k2 = k;
o[k2] = m[k];
}));
var __setModuleDefault = (this && this.__setModuleDefault) || (Object.create ? (function(o, v) {
Object.defineProperty(o, "default", { enumerable: true, value: v });
}) : function(o, v) {
o["default"] = v;
});
var __importStar = (this && this.__importStar) || function (mod) {
if (mod && mod.__esModule) return mod;
var result = {};
if (mod != null) for (var k in mod) if (k !== "default" && Object.prototype.hasOwnProperty.call(mod, k)) __createBinding(result, mod, k);
__setModuleDefault(result, mod);
return result;
};
var __importDefault = (this && this.__importDefault) || function (mod) {
return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", ({ value: true }));
const core = __importStar(__nccwpck_require__(2186));
const github = __importStar(__nccwpck_require__(5438));
const fs_1 = __importDefault(__nccwpck_require__(7147));
const run = () => {
// Prefix path to the graphite metric
const GRAPHITE_PATH = 'reassure';
let regressionOutput;
try {
regressionOutput = JSON.parse(fs_1.default.readFileSync('.reassure/output.json', 'utf8'));
}
catch (err) {
// Handle errors that occur during file reading or parsing
if (err instanceof Error) {
console.error('Error while parsing output.json:', err.message);
core.setFailed(err);
}
}
const creationDate = regressionOutput.metadata.current.creationDate;
const timestampInMili = new Date(creationDate).getTime();
// Graphite accepts timestamp in seconds
const timestamp = Math.floor(timestampInMili / 1000);
// Get PR number from the github context
const prNumber = github.context.payload.pull_request?.number;
// We need to combine all tests from the 4 buckets
const reassureTests = [...regressionOutput.meaningless, ...regressionOutput.significant, ...regressionOutput.countChanged, ...regressionOutput.added];
// Map through every test and create string for meanDuration and meanCount
// eslint-disable-next-line rulesdir/prefer-underscore-method
const graphiteString = reassureTests
.map((test) => {
const current = test.current;
// Graphite doesn't accept metrics name with space, we replace spaces with "-"
const formattedName = current.name.split(' ').join('-');
const renderDurationString = `${GRAPHITE_PATH}.${formattedName}.renderDuration ${current.meanDuration} ${timestamp}`;
const renderCountString = `${GRAPHITE_PATH}.${formattedName}.renderCount ${current.meanCount} ${timestamp}`;
const renderPRNumberString = `${GRAPHITE_PATH}.${formattedName}.prNumber ${prNumber} ${timestamp}`;
return `${renderDurationString}\n${renderCountString}\n${renderPRNumberString}`;
})
.join('\n');
// Set generated graphite string to the github variable
core.setOutput('GRAPHITE_STRING', graphiteString);
};
if (require.main === require.cache[eval('__filename')]) {
run();
}
exports["default"] = run;


/***/ }),

/***/ 2877:
Expand Down Expand Up @@ -9745,7 +9765,7 @@ module.exports = JSON.parse('[[[0,44],"disallowed_STD3_valid"],[[45,46],"valid"]
/******/ // startup
/******/ // Load entry module and return exports
/******/ // This entry module is referenced by other modules so it can't be inlined
/******/ var __webpack_exports__ = __nccwpck_require__(1302);
/******/ var __webpack_exports__ = __nccwpck_require__(7717);
/******/ module.exports = __webpack_exports__;
/******/
/******/ })()
Expand Down
4 changes: 2 additions & 2 deletions .github/scripts/buildActions.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ declare -r GITHUB_ACTIONS=(
"$ACTIONS_DIR/authorChecklist/authorChecklist.ts"
"$ACTIONS_DIR/reviewerChecklist/reviewerChecklist.js"
"$ACTIONS_DIR/validateReassureOutput/validateReassureOutput.js"
"$ACTIONS_DIR/getGraphiteString/getGraphiteString.js"
"$ACTIONS_DIR/getArtifactInfo/getArtifactInfo.js"
"$ACTIONS_DIR/getGraphiteString/getGraphiteString.ts"
"$ACTIONS_DIR/getArtifactInfo/getArtifactInfo.ts"
)

# This will be inserted at the top of all compiled files as a warning to devs.
Expand Down
Loading