Skip to content

Add performance metrics to Autobahn test parser #495

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

Merged
merged 2 commits into from
Oct 6, 2025
Merged

Add performance metrics to Autobahn test parser #495

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea3d9df
Add performance metrics tracking to Autobahn test parser
Oct 6, 2025
63b4645
Address Gemini code review feedback
Oct 6, 2025
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
84 changes: 80 additions & 4 deletions test/autobahn/parse-results.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,26 @@ function parseResults() {
failedTests: [],
nonStrictTests: [],
unimplementedTests: [],
informationalTests: []
informationalTests: [],
performance: {
totalDuration: 0,
testCount: 0,
byCategory: {
'limits': { tests: [], totalDuration: 0, description: '9.x - Limits/Performance' },
'largeMessages': { tests: [], totalDuration: 0, description: '10.x - Large Messages' },
'fragmentation': { tests: [], totalDuration: 0, description: '12.x - WebSocket Fragmentation' },
'other': { tests: [], totalDuration: 0, description: 'Other Tests' }
}
}
};

// Parse each test case
for (const [testCase, result] of Object.entries(testResults)) {
summary.total++;

const behavior = result.behavior;
const behaviorClose = result.behaviorClose;

if (behavior === 'OK' && behaviorClose === 'OK') {
summary.ok++;
} else if (behavior === 'UNIMPLEMENTED') {
Expand Down Expand Up @@ -81,6 +91,31 @@ function parseResults() {
remoteCloseCode: result.remoteCloseCode
});
}

// Track performance metrics
if (result.duration !== undefined) {
summary.performance.totalDuration += result.duration;
summary.performance.testCount++;

// Categorize performance tests
const majorCategory = testCase.split('.')[0];
let category = 'other';

if (majorCategory === '9') {
category = 'limits';
} else if (majorCategory === '10') {
category = 'largeMessages';
} else if (majorCategory === '12') {
category = 'fragmentation';
}
Copy link

@gemini-code-assist gemini-code-assist bot Oct 6, 2025

Choose a reason for hiding this comment

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

medium

This if/else if chain works, but using a map object would be more concise and easier to maintain if more categories are added in the future. For optimal performance, this map could be defined once outside the loop.

      const categoryMap = {
        '9': 'limits',
        '10': 'largeMessages',
        '12': 'fragmentation'
      };
      const category = categoryMap[majorCategory] || 'other';


summary.performance.byCategory[category].tests.push({
case: testCase,
Copy link

@gemini-code-assist gemini-code-assist bot Oct 6, 2025

Choose a reason for hiding this comment

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

medium

case is a reserved keyword in JavaScript. While it's allowed as a property name in object literals, it's best practice to avoid it to prevent potential confusion, conflicts with linters, or issues with language features like destructuring with reserved words. I suggest renaming it to testCase for clarity and safety.

        testCase: testCase,

duration: result.duration,
description: result.description
});
summary.performance.byCategory[category].totalDuration += result.duration;
}
}

// Print summary
Expand Down Expand Up @@ -144,7 +179,48 @@ function parseResults() {
}
}

console.log('\n');
// Print performance summary
if (summary.performance.testCount > 0) {
console.log('=== PERFORMANCE METRICS ===');
console.log(` Total test duration: ${summary.performance.totalDuration.toLocaleString()}ms`);
console.log(` Tests with timing data: ${summary.performance.testCount}`);
console.log(` Average duration: ${(summary.performance.totalDuration / summary.performance.testCount).toFixed(2)}ms\n`);

// Print category breakdown for performance-focused tests
const perfCategories = ['limits', 'largeMessages', 'fragmentation'];
Copy link

@gemini-code-assist gemini-code-assist bot Oct 6, 2025

Choose a reason for hiding this comment

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

medium

Hardcoding the perfCategories array makes the code less maintainable. If you add or remove categories from the summary.performance.byCategory object, you'll have to remember to update this array as well. It would be more robust to generate this array dynamically from the object's keys, excluding 'other'.

    const perfCategories = Object.keys(summary.performance.byCategory).filter(key => key !== 'other');

let hasPerfData = false;

for (const categoryKey of perfCategories) {
const category = summary.performance.byCategory[categoryKey];
if (category.tests.length > 0) {
hasPerfData = true;
const avgDuration = (category.totalDuration / category.tests.length).toFixed(2);
console.log(` ${category.description}:`);
console.log(` Tests: ${category.tests.length}`);
console.log(` Total duration: ${category.totalDuration.toLocaleString()}ms`);
console.log(` Average duration: ${avgDuration}ms`);

// Show top 5 slowest tests in this category
const slowestTests = category.tests
.sort((a, b) => b.duration - a.duration)
.slice(0, 5);
Copy link

@gemini-code-assist gemini-code-assist bot Oct 6, 2025

Choose a reason for hiding this comment

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

high

The sort() method modifies the array in-place. This means you are mutating the category.tests array within your summary object, which is an unintended side effect that could cause bugs if other parts of the application expect the test data to be in its original order. It's safer to create a copy of the array before sorting.

        const slowestTests = [...category.tests]
          .sort((a, b) => b.duration - a.duration)
          .slice(0, 5);


if (slowestTests.length > 0) {
console.log(' Slowest tests:');
slowestTests.forEach(test => {
console.log(` ${test.case}: ${test.duration}ms`);
});
}
console.log('');
}
}

if (!hasPerfData) {
console.log(' No performance-focused tests (9.x, 10.x, 12.x) executed.\n');
}
}

console.log('');

// Exit with error code if there are actual failures
if (summary.failed > 0) {
Expand Down