Skip to content
Open
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
221 changes: 203 additions & 18 deletions TestResultSummaryService/Database.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,13 @@ class Database {
}

// ToDo: impl check can be added once we have impl stored
async getTotals(query) {
/**
* Base function for aggregating data from testResults.
* @param {Object} query - The query object.
* @param {string} type - The type of request (e.g., 'totals', 'rerunDetails', 'jobsDetails').
* @returns {Object} - The aggregation result.
*/
async testResultsBaseAggregation(query, type) {
const url = query.url;
const buildName = query.buildName;
let id = query.id;
Expand Down Expand Up @@ -159,27 +165,61 @@ class Database {
if (query.platform)
buildNameRegex = `${buildNameRegex}${query.platform}`;

const result = await this.aggregate([
{ $match: matchQuery },
{
$graphLookup: {
from: 'testResults',
startWith: '$_id',
connectFromField: '_id',
connectToField: 'parentId',
as: 'childBuilds',
// Call the routing function to get the specific aggregation
const specificAggregation = this.getSpecificAggregation(type, buildNameRegex, query);

try {
const result = await this.aggregate([
{ $match: matchQuery },
{
$graphLookup: {
from: 'testResults',
startWith: '$_id',
connectFromField: '_id',
connectToField: 'parentId',
as: 'childBuilds',
},
},
},
{
$project: {
childBuilds: '$childBuilds',
{
$project: {
childBuilds: '$childBuilds',
},
},
},
...specificAggregation,
]);
return result[0] || {};
} catch (error) {
console.error('Error:', error);
}
}

/**
* Routing function to get the specific aggregation based on the type.
* @param {string} type - The type of request (e.g., 'totals', 'rerunDetails', 'jobsDetails').
* @param {string} buildNameRegex - The build name regex.
* @param {Object} query - The query object.
* @returns {Array} - The specific aggregation steps.
*/
getSpecificAggregation(type, buildNameRegex, query) {
switch (type) {
case 'totals':
return this.getTotalsAggregation(query, buildNameRegex);
case 'rerunDetails':
return this.getRerunDetailsAggregation();
case 'jobsDetails':
return this.getJobsDetailsAggregation();
default:
throw new Error(`Unknown type: ${type}`);
}
}
Comment on lines +203 to +214
Copy link
Contributor

Choose a reason for hiding this comment

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

getSpecificAggregation() calls different functions based on the switch type. Each function has its own mandate and purpose. I do not think we should create getSpecificAggregation() and use the switch case here.

If you really want to utilize the common code in testResultsBaseAggregation(), pass specificAggregation in testResultsBaseAggregation().


getTotalsAggregation(query, buildNameRegex) {
return [
{ $unwind: '$childBuilds' },
{ $match: { 'childBuilds.buildName': { $regex: buildNameRegex } } },
{
$group: {
_id: id,
_id: query.id,
total: { $sum: '$childBuilds.testSummary.total' },
executed: { $sum: '$childBuilds.testSummary.executed' },
passed: { $sum: '$childBuilds.testSummary.passed' },
Expand All @@ -188,8 +228,153 @@ class Database {
skipped: { $sum: '$childBuilds.testSummary.skipped' },
},
},
]);
return result[0] || {};
];
}

getRerunDetailsAggregation() {
return [
{
$addFields: {
manual_rerun_needed_list: {
$filter: {
input: '$childBuilds',
as: 'build',
cond: {
$and: [
{ $in: ['$$build.buildResult', ['UNSTABLE', 'FAILED', 'ABORTED']] },
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think there is FAILED state in buildResult. It should be FAILURE.

{
$or: [
{
$regexMatch: { input: '$$build.buildName', regex: /_rerun$/ }
},
{
// If buildName does not end with '_rerun', check if another build with the same name exists that ends with '_rerun'
$and: [
{ $not: [{ $regexMatch: { input: '$$build.buildName', regex: /_rerun$/ } }] },
{
// Veriy that there is no '_rerun' build for this build
$eq: [
{
$size: {
$filter: {
input: '$childBuilds',
as: 'internal_build',
cond: {
$and: [
{
$regexMatch: {
input: '$$internal_build.buildName',
regex: { $concat: ['^', '$$build.buildName'] }
}
},
{ $regexMatch: { input: '$$internal_build.buildName', regex: /_rerun$/ } }
]
}
}
}
},
0
]
}
]
}
]
}
]
},
}
}
},
},
{
$addFields: {
manual_rerun_needed: {
$size:
'$manual_rerun_needed_list'
}
},
},
{
$addFields: {
tests_needed_manual_rerun: {
$reduce: {
input: '$manual_rerun_needed_list',
initialValue: 0,
in: {
$add: [
'$$value',
{ $ifNull: ['$$this.testSummary.failed', 0] }
]
}
}
}
}
},
{
$addFields: {
manual_rerun_needed_regex: {
$concat: [
'^(',
{
$reduce: {
input: '$manual_rerun_needed_list',
initialValue: '',
in: {
$cond: [
{ $eq: ['$$value', ''] }, // If the first item
{ $replaceAll: { input: '$$this.buildName', find: '.', replacement: '\\.' } },
{
$concat: [
'$$value',
'|',
{ $replaceAll: { input: '$$this.buildName', find: '.', replacement: '\\.' } }
]
}
]
}
}
},
')$'
]
}
}
},
{ $unset: ['childBuilds', 'manual_rerun_needed_list'] }
];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is complex to review and test. Could you write a pipeline query?

Key Performance Improvements

  • Avoids multiple $filter operations by pre-filtering failed builds.
  • Replaces $reduce with $sum in $lookup, making it more efficient.
  • Minimizes $regexMatch usage, reducing processing time.
  • Efficient regex generation using $group, avoiding multiple conditionals.


getJobsDetailsAggregation() {
return [
{
$addFields: {
job_success_rate: {
$round: [
{
$multiply: [
{
$divide: [
{
$size: {
$filter: {
input: "$childBuilds",
as: "build",
cond: { $eq: ["$$build.buildResult", "SUCCESS"] }
}
}
},
{ $size: "$childBuilds" }
]
},
100
]
},
2
]
}
}
},
{ $unset: ['childBuilds'] }
];
Comment on lines +346 to +377
Copy link
Contributor

Choose a reason for hiding this comment

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

If you do calculations in JS, your code should be much simpler.

}

async getRootBuildId(id) {
Expand Down
58 changes: 58 additions & 0 deletions TestResultSummaryService/routes/GetFailedTestByMachine.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
const { TestResultsDB } = require('../Database');

module.exports = async (req, res) => {
try {
const { parentId } = req.query;

if (!parentId) {
return res.status(400).send({ error: 'parentId is required' });
}

const db = new TestResultsDB();

// Fetch all tests and their machines for the specified parentId
const buildData = await db.getSpecificData(
{ parentId: parentId },
{ tests: 1, machine: 1 }
);

if (!buildData || buildData.length === 0) {
return res
.status(404)
.send({ error: 'No builds found for the given parentId' });
}

// Initialize a map to store machine names and their respective failed test counts
const failedTestsByMachine = {};

// Iterate through all builds and process tests
for (const build of buildData) {
const { tests, machine } = build;

if (tests && Array.isArray(tests)) {
for (const test of tests) {
if (test.testResult === 'FAILED') {
// Accumulate failure count for the machine
failedTestsByMachine[machine] =
(failedTestsByMachine[machine] || 0) + 1;
}
}
}
}

// Format the result as an array of dictionaries with "machine" and "failedTest"
const formattedResult = Object.entries(failedTestsByMachine)
.map(([machine, failedTests]) => ({
machine,
failedTests,
}))
.sort((a, b) => b.failedTests - a.failedTests) // Sort in descending order
.slice(0, 3); // Keep only the top 3 machines

// Send the formatted result as the response
res.send(formattedResult);
} catch (error) {
console.error('Error processing request:', error);
res.status(500).send({ error: 'Internal Server Error' });
}
};
18 changes: 18 additions & 0 deletions TestResultSummaryService/routes/getJobsDetails.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
const { TestResultsDB } = require('../Database');

/**
* getJobsDetails returns jobs details
* @route GET /api/getJobsDetails
* @group Test - Operations about test
* @param {number} id Optional.
* @param {string} url Optional. If provided, it has to be used with buildName and buildNum
* @param {string} buildName Optional. If provided, it has to be used with url and buildNum
* @param {string} buildNum Optional. If provided, it has to be used with url and buildName
* @return {object} {job_success_rate}
*/

module.exports = async (req, res) => {
const testResultsDB = new TestResultsDB();
const result = await testResultsDB.testResultsBaseAggregation(req.query, 'jobsDetails');
res.send(result);
};
18 changes: 18 additions & 0 deletions TestResultSummaryService/routes/getRerunDetails.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
const { TestResultsDB } = require('../Database');

/**
* getRerunDetails returns rerun summary
* @route GET /api/getRerunDetails
* @group Test - Operations about test
* @param {number} id Optional.
* @param {string} url Optional. If provided, it has to be used with buildName and buildNum
* @param {string} buildName Optional. If provided, it has to be used with url and buildNum
* @param {string} buildNum Optional. If provided, it has to be used with url and buildName
* @return {object} {manual_rerun_needed, tests_needed_manual_rerun, manual_rerun_needed_regex}
*/

module.exports = async (req, res) => {
const testResultsDB = new TestResultsDB();
const result = await testResultsDB.testResultsBaseAggregation(req.query, 'rerunDetails');
res.send(result);
};
2 changes: 1 addition & 1 deletion TestResultSummaryService/routes/getTotals.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ const { TestResultsDB, ObjectID } = require('../Database');

module.exports = async (req, res) => {
const testResultsDB = new TestResultsDB();
const result = await testResultsDB.getTotals(req.query);
const result = await testResultsDB.testResultsBaseAggregation(req.query, 'totals');
res.send(result);
};
3 changes: 3 additions & 0 deletions TestResultSummaryService/routes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ app.get('/getFeedbackUrl', require('./getFeedbackUrl'));
app.get('/rescanBuild', require('./rescanBuild'));
app.get('/testParserViaFile', require('./test/testParserViaFile'));
app.get('/testParserViaLogStream', require('./test/testParserViaLogStream'));
app.get('/getRerunDetails', require('./getRerunDetails'));
app.get('/getJobsDetails', require('./getJobsDetails'));
app.get('/GetFailedTestByMachine', require('./GetFailedTestByMachine'));

app.get('/updateComments', require('./updateComments'));
app.get('/updateKeepForever', require('./updateKeepForever'));
Expand Down
4 changes: 2 additions & 2 deletions test-result-summary-client/src/Build/BuildLink.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const BuildLink = ({
if (id) {
return (
<span>
{label}
<strong>{label}</strong>
<Link
to={{
pathname: '/buildDetail',
Expand All @@ -33,4 +33,4 @@ const BuildLink = ({
return null;
};

export default BuildLink;
export default BuildLink;
Loading
Loading