Add load tests for expansion analyzers - #755

Open
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests
Open

Add load tests for expansion analyzers#755
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests

Conversation

CopilotAI commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

Expansion analyzers can take excessive time on large files. This adds load tests to catch performance regressions.

Changes

  • test/analyzer-load-test.rkt: Load tests for all four expansion analyzers (identifier-usage, variable-mutability, ignored-result-values, function-expression)
    • Generates ~300 line test program with functions, variables, mutations, and nested expressions
    • Uses expansion-analyze directly with time-apply to measure actual analyzer performance
    • Pre-expands syntax once using source-expand and reuses it for all tests
    • Verifies each analyzer completes within 30 second timeout
    • All tests are in a (module+ test ...) submodule

Each test measures elapsed time and fails with diagnostic message if timeout exceeded:

(test-case "identifier-usage-analyzer load test"
(define-values (result cpu-time real-time gc-time)
(time-apply expansion-analyze (list identifier-usage-analyzer expanded-syntax)))
(check-true (< real-time timeout-ms)
(format "identifier-usage-analyzer took ~a ms, expected < ~a ms"
real-time timeout-ms)))
Original prompt

This section details on the original issue you should resolve

<issue_title>Analyzer load tests</issue_title>
<issue_description>Expansion analyzers can easily take much longer than expected on large files. There should be a load test for each analyzer that asserts that the analyzer is capable of analyzing a suitably large file within a reasonable amount of time. This should use the analyzer timeout configuration option mentioned in #733.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
CopilotAI changed the title [WIP] Add load tests for each analyzer performanceAdd load tests for expansion analyzersNov 13, 2025
CopilotAI requested a review from jackfirthNovember 13, 2025 17:25
@jackfirth
jackfirth marked this pull request as ready for review November 14, 2025 00:22

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resyntax analyzed 1 file in this pull request and found no issues.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 93.71% (+0.05%) from 93.661%
when pulling 06f12ac on copilot/add-analyzer-load-tests
into ef1a5e9 on master.

@jackfirthjackfirth left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Put all of this inside of a (module+ test ...) submodule.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done in 1ab6849. All code is now inside (module+ test ...).

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +89 to +92
(define analysis
(source-analyze large-test-source
#:analyzers (list variable-mutability-analyzer)
#:timeout-ms timeout-ms))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This doesn't work. The source-analyze will kill the analyzer after timeout-ms and return an empty source-analysis? value while logging a warning. So the test cases in this file will always pass.

Instead of using source-analyze, call the expansion analyzer directly (with expansion-analyze) by passing it fully-expanded syntax created using source-expand in a new base namespace. Use time-apply to measure how long the analyzer takes to process the expanded syntax.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed in 1ab6849. Now using expansion-analyze directly with time-apply to measure actual analyzer performance. The syntax is expanded once using source-expand and reused for all tests.

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +142 to +165
(test-case "all analyzers together load test"
;; This test ensures all analyzers can work together on a large file
;; within the timeout period.
(define all-analyzers
(list identifier-usage-analyzer
variable-mutability-analyzer
ignored-result-values-analyzer
function-expression-analyzer))

(define start-time (current-inexact-milliseconds))
(define analysis
(source-analyze large-test-source
#:analyzers all-analyzers
#:timeout-ms timeout-ms))
(define elapsed-ms (- (current-inexact-milliseconds) start-time))

;; Verify the analysis completed successfully
(check-true (source-code-analysis? analysis))

;; Verify it completed within the timeout (with some margin)
(check-true (< elapsed-ms timeout-ms)
(format "all analyzers together took ~a ms, expected < ~a ms"
elapsed-ms
timeout-ms)))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't bother with this load test.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Removed in 1ab6849.

- Wrap everything in (module+ test ...) submodule
- Use expansion-analyze directly instead of source-analyze
- Use time-apply to measure timing
- Expand syntax once and reuse for all tests
- Remove "all analyzers together" test
Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Analyzer load tests

3 participants

@coveralls@jackfirth
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Add load tests for expansion analyzers - #755

Open
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests
Open

Add load tests for expansion analyzers#755
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests

Conversation

CopilotAI commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

Expansion analyzers can take excessive time on large files. This adds load tests to catch performance regressions.

Changes

  • test/analyzer-load-test.rkt: Load tests for all four expansion analyzers (identifier-usage, variable-mutability, ignored-result-values, function-expression)
    • Generates ~300 line test program with functions, variables, mutations, and nested expressions
    • Uses expansion-analyze directly with time-apply to measure actual analyzer performance
    • Pre-expands syntax once using source-expand and reuses it for all tests
    • Verifies each analyzer completes within 30 second timeout
    • All tests are in a (module+ test ...) submodule

Each test measures elapsed time and fails with diagnostic message if timeout exceeded:

(test-case "identifier-usage-analyzer load test"
(define-values (result cpu-time real-time gc-time)
(time-apply expansion-analyze (list identifier-usage-analyzer expanded-syntax)))
(check-true (< real-time timeout-ms)
(format "identifier-usage-analyzer took ~a ms, expected < ~a ms"
real-time timeout-ms)))
Original prompt

This section details on the original issue you should resolve

<issue_title>Analyzer load tests</issue_title>
<issue_description>Expansion analyzers can easily take much longer than expected on large files. There should be a load test for each analyzer that asserts that the analyzer is capable of analyzing a suitably large file within a reasonable amount of time. This should use the analyzer timeout configuration option mentioned in #733.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
CopilotAI changed the title [WIP] Add load tests for each analyzer performanceAdd load tests for expansion analyzersNov 13, 2025
CopilotAI requested a review from jackfirthNovember 13, 2025 17:25
@jackfirth
jackfirth marked this pull request as ready for review November 14, 2025 00:22

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resyntax analyzed 1 file in this pull request and found no issues.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 93.71% (+0.05%) from 93.661%
when pulling 06f12ac on copilot/add-analyzer-load-tests
into ef1a5e9 on master.

@jackfirthjackfirth left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Put all of this inside of a (module+ test ...) submodule.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done in 1ab6849. All code is now inside (module+ test ...).

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +89 to +92
(define analysis
(source-analyze large-test-source
#:analyzers (list variable-mutability-analyzer)
#:timeout-ms timeout-ms))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This doesn't work. The source-analyze will kill the analyzer after timeout-ms and return an empty source-analysis? value while logging a warning. So the test cases in this file will always pass.

Instead of using source-analyze, call the expansion analyzer directly (with expansion-analyze) by passing it fully-expanded syntax created using source-expand in a new base namespace. Use time-apply to measure how long the analyzer takes to process the expanded syntax.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed in 1ab6849. Now using expansion-analyze directly with time-apply to measure actual analyzer performance. The syntax is expanded once using source-expand and reused for all tests.

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +142 to +165
(test-case "all analyzers together load test"
;; This test ensures all analyzers can work together on a large file
;; within the timeout period.
(define all-analyzers
(list identifier-usage-analyzer
variable-mutability-analyzer
ignored-result-values-analyzer
function-expression-analyzer))

(define start-time (current-inexact-milliseconds))
(define analysis
(source-analyze large-test-source
#:analyzers all-analyzers
#:timeout-ms timeout-ms))
(define elapsed-ms (- (current-inexact-milliseconds) start-time))

;; Verify the analysis completed successfully
(check-true (source-code-analysis? analysis))

;; Verify it completed within the timeout (with some margin)
(check-true (< elapsed-ms timeout-ms)
(format "all analyzers together took ~a ms, expected < ~a ms"
elapsed-ms
timeout-ms)))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't bother with this load test.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Removed in 1ab6849.

- Wrap everything in (module+ test ...) submodule
- Use expansion-analyze directly instead of source-analyze
- Use time-apply to measure timing
- Expand syntax once and reuse for all tests
- Remove "all analyzers together" test
Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Analyzer load tests

3 participants

@coveralls@jackfirth
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add load tests for expansion analyzers - #755

Open
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests
Open

Add load tests for expansion analyzers#755
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests

Conversation

CopilotAI commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

Expansion analyzers can take excessive time on large files. This adds load tests to catch performance regressions.

Changes

  • test/analyzer-load-test.rkt: Load tests for all four expansion analyzers (identifier-usage, variable-mutability, ignored-result-values, function-expression)
    • Generates ~300 line test program with functions, variables, mutations, and nested expressions
    • Uses expansion-analyze directly with time-apply to measure actual analyzer performance
    • Pre-expands syntax once using source-expand and reuses it for all tests
    • Verifies each analyzer completes within 30 second timeout
    • All tests are in a (module+ test ...) submodule

Each test measures elapsed time and fails with diagnostic message if timeout exceeded:

(test-case "identifier-usage-analyzer load test"
(define-values (result cpu-time real-time gc-time)
(time-apply expansion-analyze (list identifier-usage-analyzer expanded-syntax)))
(check-true (< real-time timeout-ms)
(format "identifier-usage-analyzer took ~a ms, expected < ~a ms"
real-time timeout-ms)))
Original prompt

This section details on the original issue you should resolve

<issue_title>Analyzer load tests</issue_title>
<issue_description>Expansion analyzers can easily take much longer than expected on large files. There should be a load test for each analyzer that asserts that the analyzer is capable of analyzing a suitably large file within a reasonable amount of time. This should use the analyzer timeout configuration option mentioned in #733.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
CopilotAI changed the title [WIP] Add load tests for each analyzer performanceAdd load tests for expansion analyzersNov 13, 2025
CopilotAI requested a review from jackfirthNovember 13, 2025 17:25
@jackfirth
jackfirth marked this pull request as ready for review November 14, 2025 00:22

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resyntax analyzed 1 file in this pull request and found no issues.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 93.71% (+0.05%) from 93.661%
when pulling 06f12ac on copilot/add-analyzer-load-tests
into ef1a5e9 on master.

@jackfirthjackfirth left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Put all of this inside of a (module+ test ...) submodule.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done in 1ab6849. All code is now inside (module+ test ...).

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +89 to +92
(define analysis
(source-analyze large-test-source
#:analyzers (list variable-mutability-analyzer)
#:timeout-ms timeout-ms))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This doesn't work. The source-analyze will kill the analyzer after timeout-ms and return an empty source-analysis? value while logging a warning. So the test cases in this file will always pass.

Instead of using source-analyze, call the expansion analyzer directly (with expansion-analyze) by passing it fully-expanded syntax created using source-expand in a new base namespace. Use time-apply to measure how long the analyzer takes to process the expanded syntax.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed in 1ab6849. Now using expansion-analyze directly with time-apply to measure actual analyzer performance. The syntax is expanded once using source-expand and reused for all tests.

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +142 to +165
(test-case "all analyzers together load test"
;; This test ensures all analyzers can work together on a large file
;; within the timeout period.
(define all-analyzers
(list identifier-usage-analyzer
variable-mutability-analyzer
ignored-result-values-analyzer
function-expression-analyzer))

(define start-time (current-inexact-milliseconds))
(define analysis
(source-analyze large-test-source
#:analyzers all-analyzers
#:timeout-ms timeout-ms))
(define elapsed-ms (- (current-inexact-milliseconds) start-time))

;; Verify the analysis completed successfully
(check-true (source-code-analysis? analysis))

;; Verify it completed within the timeout (with some margin)
(check-true (< elapsed-ms timeout-ms)
(format "all analyzers together took ~a ms, expected < ~a ms"
elapsed-ms
timeout-ms)))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't bother with this load test.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Removed in 1ab6849.

- Wrap everything in (module+ test ...) submodule
- Use expansion-analyze directly instead of source-analyze
- Use time-apply to measure timing
- Expand syntax once and reuse for all tests
- Remove "all analyzers together" test
Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Analyzer load tests

3 participants

@coveralls@jackfirth
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add load tests for expansion analyzers - #755

Open
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests
Open

Add load tests for expansion analyzers#755
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests

Conversation

CopilotAI commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

Expansion analyzers can take excessive time on large files. This adds load tests to catch performance regressions.

Changes

  • test/analyzer-load-test.rkt: Load tests for all four expansion analyzers (identifier-usage, variable-mutability, ignored-result-values, function-expression)
    • Generates ~300 line test program with functions, variables, mutations, and nested expressions
    • Uses expansion-analyze directly with time-apply to measure actual analyzer performance
    • Pre-expands syntax once using source-expand and reuses it for all tests
    • Verifies each analyzer completes within 30 second timeout
    • All tests are in a (module+ test ...) submodule

Each test measures elapsed time and fails with diagnostic message if timeout exceeded:

(test-case "identifier-usage-analyzer load test"
(define-values (result cpu-time real-time gc-time)
(time-apply expansion-analyze (list identifier-usage-analyzer expanded-syntax)))
(check-true (< real-time timeout-ms)
(format "identifier-usage-analyzer took ~a ms, expected < ~a ms"
real-time timeout-ms)))
Original prompt

This section details on the original issue you should resolve

<issue_title>Analyzer load tests</issue_title>
<issue_description>Expansion analyzers can easily take much longer than expected on large files. There should be a load test for each analyzer that asserts that the analyzer is capable of analyzing a suitably large file within a reasonable amount of time. This should use the analyzer timeout configuration option mentioned in #733.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
CopilotAI changed the title [WIP] Add load tests for each analyzer performanceAdd load tests for expansion analyzersNov 13, 2025
CopilotAI requested a review from jackfirthNovember 13, 2025 17:25
@jackfirth
jackfirth marked this pull request as ready for review November 14, 2025 00:22

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resyntax analyzed 1 file in this pull request and found no issues.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 93.71% (+0.05%) from 93.661%
when pulling 06f12ac on copilot/add-analyzer-load-tests
into ef1a5e9 on master.

@jackfirthjackfirth left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Put all of this inside of a (module+ test ...) submodule.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done in 1ab6849. All code is now inside (module+ test ...).

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +89 to +92
(define analysis
(source-analyze large-test-source
#:analyzers (list variable-mutability-analyzer)
#:timeout-ms timeout-ms))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This doesn't work. The source-analyze will kill the analyzer after timeout-ms and return an empty source-analysis? value while logging a warning. So the test cases in this file will always pass.

Instead of using source-analyze, call the expansion analyzer directly (with expansion-analyze) by passing it fully-expanded syntax created using source-expand in a new base namespace. Use time-apply to measure how long the analyzer takes to process the expanded syntax.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed in 1ab6849. Now using expansion-analyze directly with time-apply to measure actual analyzer performance. The syntax is expanded once using source-expand and reused for all tests.

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +142 to +165
(test-case "all analyzers together load test"
;; This test ensures all analyzers can work together on a large file
;; within the timeout period.
(define all-analyzers
(list identifier-usage-analyzer
variable-mutability-analyzer
ignored-result-values-analyzer
function-expression-analyzer))

(define start-time (current-inexact-milliseconds))
(define analysis
(source-analyze large-test-source
#:analyzers all-analyzers
#:timeout-ms timeout-ms))
(define elapsed-ms (- (current-inexact-milliseconds) start-time))

;; Verify the analysis completed successfully
(check-true (source-code-analysis? analysis))

;; Verify it completed within the timeout (with some margin)
(check-true (< elapsed-ms timeout-ms)
(format "all analyzers together took ~a ms, expected < ~a ms"
elapsed-ms
timeout-ms)))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't bother with this load test.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Removed in 1ab6849.

- Wrap everything in (module+ test ...) submodule
- Use expansion-analyze directly instead of source-analyze
- Use time-apply to measure timing
- Expand syntax once and reuse for all tests
- Remove "all analyzers together" test
Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Analyzer load tests

3 participants

@coveralls@jackfirth
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Add load tests for expansion analyzers - #755

Open
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests
Open

Add load tests for expansion analyzers#755
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests

Conversation

CopilotAI commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

Expansion analyzers can take excessive time on large files. This adds load tests to catch performance regressions.

Changes

  • test/analyzer-load-test.rkt: Load tests for all four expansion analyzers (identifier-usage, variable-mutability, ignored-result-values, function-expression)
    • Generates ~300 line test program with functions, variables, mutations, and nested expressions
    • Uses expansion-analyze directly with time-apply to measure actual analyzer performance
    • Pre-expands syntax once using source-expand and reuses it for all tests
    • Verifies each analyzer completes within 30 second timeout
    • All tests are in a (module+ test ...) submodule

Each test measures elapsed time and fails with diagnostic message if timeout exceeded:

(test-case "identifier-usage-analyzer load test"
(define-values (result cpu-time real-time gc-time)
(time-apply expansion-analyze (list identifier-usage-analyzer expanded-syntax)))
(check-true (< real-time timeout-ms)
(format "identifier-usage-analyzer took ~a ms, expected < ~a ms"
real-time timeout-ms)))
Original prompt

This section details on the original issue you should resolve

<issue_title>Analyzer load tests</issue_title>
<issue_description>Expansion analyzers can easily take much longer than expected on large files. There should be a load test for each analyzer that asserts that the analyzer is capable of analyzing a suitably large file within a reasonable amount of time. This should use the analyzer timeout configuration option mentioned in #733.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
CopilotAI changed the title [WIP] Add load tests for each analyzer performanceAdd load tests for expansion analyzersNov 13, 2025
CopilotAI requested a review from jackfirthNovember 13, 2025 17:25
@jackfirth
jackfirth marked this pull request as ready for review November 14, 2025 00:22

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resyntax analyzed 1 file in this pull request and found no issues.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 93.71% (+0.05%) from 93.661%
when pulling 06f12ac on copilot/add-analyzer-load-tests
into ef1a5e9 on master.

@jackfirthjackfirth left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Put all of this inside of a (module+ test ...) submodule.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done in 1ab6849. All code is now inside (module+ test ...).

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +89 to +92
(define analysis
(source-analyze large-test-source
#:analyzers (list variable-mutability-analyzer)
#:timeout-ms timeout-ms))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This doesn't work. The source-analyze will kill the analyzer after timeout-ms and return an empty source-analysis? value while logging a warning. So the test cases in this file will always pass.

Instead of using source-analyze, call the expansion analyzer directly (with expansion-analyze) by passing it fully-expanded syntax created using source-expand in a new base namespace. Use time-apply to measure how long the analyzer takes to process the expanded syntax.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed in 1ab6849. Now using expansion-analyze directly with time-apply to measure actual analyzer performance. The syntax is expanded once using source-expand and reused for all tests.

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +142 to +165
(test-case "all analyzers together load test"
;; This test ensures all analyzers can work together on a large file
;; within the timeout period.
(define all-analyzers
(list identifier-usage-analyzer
variable-mutability-analyzer
ignored-result-values-analyzer
function-expression-analyzer))

(define start-time (current-inexact-milliseconds))
(define analysis
(source-analyze large-test-source
#:analyzers all-analyzers
#:timeout-ms timeout-ms))
(define elapsed-ms (- (current-inexact-milliseconds) start-time))

;; Verify the analysis completed successfully
(check-true (source-code-analysis? analysis))

;; Verify it completed within the timeout (with some margin)
(check-true (< elapsed-ms timeout-ms)
(format "all analyzers together took ~a ms, expected < ~a ms"
elapsed-ms
timeout-ms)))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't bother with this load test.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Removed in 1ab6849.

- Wrap everything in (module+ test ...) submodule
- Use expansion-analyze directly instead of source-analyze
- Use time-apply to measure timing
- Expand syntax once and reuse for all tests
- Remove "all analyzers together" test
Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Analyzer load tests

3 participants

@coveralls@jackfirth
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add load tests for expansion analyzers - #755

Open
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests
Open

Add load tests for expansion analyzers#755
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests

Conversation

CopilotAI commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

Expansion analyzers can take excessive time on large files. This adds load tests to catch performance regressions.

Changes

  • test/analyzer-load-test.rkt: Load tests for all four expansion analyzers (identifier-usage, variable-mutability, ignored-result-values, function-expression)
    • Generates ~300 line test program with functions, variables, mutations, and nested expressions
    • Uses expansion-analyze directly with time-apply to measure actual analyzer performance
    • Pre-expands syntax once using source-expand and reuses it for all tests
    • Verifies each analyzer completes within 30 second timeout
    • All tests are in a (module+ test ...) submodule

Each test measures elapsed time and fails with diagnostic message if timeout exceeded:

(test-case "identifier-usage-analyzer load test"
(define-values (result cpu-time real-time gc-time)
(time-apply expansion-analyze (list identifier-usage-analyzer expanded-syntax)))
(check-true (< real-time timeout-ms)
(format "identifier-usage-analyzer took ~a ms, expected < ~a ms"
real-time timeout-ms)))
Original prompt

This section details on the original issue you should resolve

<issue_title>Analyzer load tests</issue_title>
<issue_description>Expansion analyzers can easily take much longer than expected on large files. There should be a load test for each analyzer that asserts that the analyzer is capable of analyzing a suitably large file within a reasonable amount of time. This should use the analyzer timeout configuration option mentioned in #733.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
CopilotAI changed the title [WIP] Add load tests for each analyzer performanceAdd load tests for expansion analyzersNov 13, 2025
CopilotAI requested a review from jackfirthNovember 13, 2025 17:25
@jackfirth
jackfirth marked this pull request as ready for review November 14, 2025 00:22

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resyntax analyzed 1 file in this pull request and found no issues.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 93.71% (+0.05%) from 93.661%
when pulling 06f12ac on copilot/add-analyzer-load-tests
into ef1a5e9 on master.

@jackfirthjackfirth left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Put all of this inside of a (module+ test ...) submodule.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done in 1ab6849. All code is now inside (module+ test ...).

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +89 to +92
(define analysis
(source-analyze large-test-source
#:analyzers (list variable-mutability-analyzer)
#:timeout-ms timeout-ms))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This doesn't work. The source-analyze will kill the analyzer after timeout-ms and return an empty source-analysis? value while logging a warning. So the test cases in this file will always pass.

Instead of using source-analyze, call the expansion analyzer directly (with expansion-analyze) by passing it fully-expanded syntax created using source-expand in a new base namespace. Use time-apply to measure how long the analyzer takes to process the expanded syntax.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed in 1ab6849. Now using expansion-analyze directly with time-apply to measure actual analyzer performance. The syntax is expanded once using source-expand and reused for all tests.

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +142 to +165
(test-case "all analyzers together load test"
;; This test ensures all analyzers can work together on a large file
;; within the timeout period.
(define all-analyzers
(list identifier-usage-analyzer
variable-mutability-analyzer
ignored-result-values-analyzer
function-expression-analyzer))

(define start-time (current-inexact-milliseconds))
(define analysis
(source-analyze large-test-source
#:analyzers all-analyzers
#:timeout-ms timeout-ms))
(define elapsed-ms (- (current-inexact-milliseconds) start-time))

;; Verify the analysis completed successfully
(check-true (source-code-analysis? analysis))

;; Verify it completed within the timeout (with some margin)
(check-true (< elapsed-ms timeout-ms)
(format "all analyzers together took ~a ms, expected < ~a ms"
elapsed-ms
timeout-ms)))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't bother with this load test.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Removed in 1ab6849.

- Wrap everything in (module+ test ...) submodule
- Use expansion-analyze directly instead of source-analyze
- Use time-apply to measure timing
- Expand syntax once and reuse for all tests
- Remove "all analyzers together" test
Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Analyzer load tests

3 participants

@coveralls@jackfirth
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add load tests for expansion analyzers - #755

Open
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests
Open

Add load tests for expansion analyzers#755
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests

Conversation

CopilotAI commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

Expansion analyzers can take excessive time on large files. This adds load tests to catch performance regressions.

Changes

  • test/analyzer-load-test.rkt: Load tests for all four expansion analyzers (identifier-usage, variable-mutability, ignored-result-values, function-expression)
    • Generates ~300 line test program with functions, variables, mutations, and nested expressions
    • Uses expansion-analyze directly with time-apply to measure actual analyzer performance
    • Pre-expands syntax once using source-expand and reuses it for all tests
    • Verifies each analyzer completes within 30 second timeout
    • All tests are in a (module+ test ...) submodule

Each test measures elapsed time and fails with diagnostic message if timeout exceeded:

(test-case "identifier-usage-analyzer load test"
(define-values (result cpu-time real-time gc-time)
(time-apply expansion-analyze (list identifier-usage-analyzer expanded-syntax)))
(check-true (< real-time timeout-ms)
(format "identifier-usage-analyzer took ~a ms, expected < ~a ms"
real-time timeout-ms)))
Original prompt

This section details on the original issue you should resolve

<issue_title>Analyzer load tests</issue_title>
<issue_description>Expansion analyzers can easily take much longer than expected on large files. There should be a load test for each analyzer that asserts that the analyzer is capable of analyzing a suitably large file within a reasonable amount of time. This should use the analyzer timeout configuration option mentioned in #733.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
CopilotAI changed the title [WIP] Add load tests for each analyzer performanceAdd load tests for expansion analyzersNov 13, 2025
CopilotAI requested a review from jackfirthNovember 13, 2025 17:25
@jackfirth
jackfirth marked this pull request as ready for review November 14, 2025 00:22

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resyntax analyzed 1 file in this pull request and found no issues.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 93.71% (+0.05%) from 93.661%
when pulling 06f12ac on copilot/add-analyzer-load-tests
into ef1a5e9 on master.

@jackfirthjackfirth left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Put all of this inside of a (module+ test ...) submodule.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done in 1ab6849. All code is now inside (module+ test ...).

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +89 to +92
(define analysis
(source-analyze large-test-source
#:analyzers (list variable-mutability-analyzer)
#:timeout-ms timeout-ms))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This doesn't work. The source-analyze will kill the analyzer after timeout-ms and return an empty source-analysis? value while logging a warning. So the test cases in this file will always pass.

Instead of using source-analyze, call the expansion analyzer directly (with expansion-analyze) by passing it fully-expanded syntax created using source-expand in a new base namespace. Use time-apply to measure how long the analyzer takes to process the expanded syntax.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed in 1ab6849. Now using expansion-analyze directly with time-apply to measure actual analyzer performance. The syntax is expanded once using source-expand and reused for all tests.

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +142 to +165
(test-case "all analyzers together load test"
;; This test ensures all analyzers can work together on a large file
;; within the timeout period.
(define all-analyzers
(list identifier-usage-analyzer
variable-mutability-analyzer
ignored-result-values-analyzer
function-expression-analyzer))

(define start-time (current-inexact-milliseconds))
(define analysis
(source-analyze large-test-source
#:analyzers all-analyzers
#:timeout-ms timeout-ms))
(define elapsed-ms (- (current-inexact-milliseconds) start-time))

;; Verify the analysis completed successfully
(check-true (source-code-analysis? analysis))

;; Verify it completed within the timeout (with some margin)
(check-true (< elapsed-ms timeout-ms)
(format "all analyzers together took ~a ms, expected < ~a ms"
elapsed-ms
timeout-ms)))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't bother with this load test.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Removed in 1ab6849.

- Wrap everything in (module+ test ...) submodule
- Use expansion-analyze directly instead of source-analyze
- Use time-apply to measure timing
- Expand syntax once and reuse for all tests
- Remove "all analyzers together" test
Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Analyzer load tests

3 participants

@coveralls@jackfirth
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Add load tests for expansion analyzers - #755

Open
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests
Open

Add load tests for expansion analyzers#755
jackfirth with Copilot wants to merge 3 commits into
masterfrom
copilot/add-analyzer-load-tests

Conversation

CopilotAI commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

Expansion analyzers can take excessive time on large files. This adds load tests to catch performance regressions.

Changes

  • test/analyzer-load-test.rkt: Load tests for all four expansion analyzers (identifier-usage, variable-mutability, ignored-result-values, function-expression)
    • Generates ~300 line test program with functions, variables, mutations, and nested expressions
    • Uses expansion-analyze directly with time-apply to measure actual analyzer performance
    • Pre-expands syntax once using source-expand and reuses it for all tests
    • Verifies each analyzer completes within 30 second timeout
    • All tests are in a (module+ test ...) submodule

Each test measures elapsed time and fails with diagnostic message if timeout exceeded:

(test-case "identifier-usage-analyzer load test"
(define-values (result cpu-time real-time gc-time)
(time-apply expansion-analyze (list identifier-usage-analyzer expanded-syntax)))
(check-true (< real-time timeout-ms)
(format "identifier-usage-analyzer took ~a ms, expected < ~a ms"
real-time timeout-ms)))
Original prompt

This section details on the original issue you should resolve

<issue_title>Analyzer load tests</issue_title>
<issue_description>Expansion analyzers can easily take much longer than expected on large files. There should be a load test for each analyzer that asserts that the analyzer is capable of analyzing a suitably large file within a reasonable amount of time. This should use the analyzer timeout configuration option mentioned in #733.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
CopilotAI changed the title [WIP] Add load tests for each analyzer performanceAdd load tests for expansion analyzersNov 13, 2025
CopilotAI requested a review from jackfirthNovember 13, 2025 17:25
@jackfirth
jackfirth marked this pull request as ready for review November 14, 2025 00:22

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resyntax analyzed 1 file in this pull request and found no issues.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 93.71% (+0.05%) from 93.661%
when pulling 06f12ac on copilot/add-analyzer-load-tests
into ef1a5e9 on master.

@jackfirthjackfirth left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Put all of this inside of a (module+ test ...) submodule.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done in 1ab6849. All code is now inside (module+ test ...).

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +89 to +92
(define analysis
(source-analyze large-test-source
#:analyzers (list variable-mutability-analyzer)
#:timeout-ms timeout-ms))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This doesn't work. The source-analyze will kill the analyzer after timeout-ms and return an empty source-analysis? value while logging a warning. So the test cases in this file will always pass.

Instead of using source-analyze, call the expansion analyzer directly (with expansion-analyze) by passing it fully-expanded syntax created using source-expand in a new base namespace. Use time-apply to measure how long the analyzer takes to process the expanded syntax.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed in 1ab6849. Now using expansion-analyze directly with time-apply to measure actual analyzer performance. The syntax is expanded once using source-expand and reused for all tests.

Comment threadtest/analyzer-load-test.rkt Outdated
Comment on lines +142 to +165
(test-case "all analyzers together load test"
;; This test ensures all analyzers can work together on a large file
;; within the timeout period.
(define all-analyzers
(list identifier-usage-analyzer
variable-mutability-analyzer
ignored-result-values-analyzer
function-expression-analyzer))

(define start-time (current-inexact-milliseconds))
(define analysis
(source-analyze large-test-source
#:analyzers all-analyzers
#:timeout-ms timeout-ms))
(define elapsed-ms (- (current-inexact-milliseconds) start-time))

;; Verify the analysis completed successfully
(check-true (source-code-analysis? analysis))

;; Verify it completed within the timeout (with some margin)
(check-true (< elapsed-ms timeout-ms)
(format "all analyzers together took ~a ms, expected < ~a ms"
elapsed-ms
timeout-ms)))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't bother with this load test.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Removed in 1ab6849.

- Wrap everything in (module+ test ...) submodule
- Use expansion-analyze directly instead of source-analyze
- Use time-apply to measure timing
- Expand syntax once and reuse for all tests
- Remove "all analyzers together" test
Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Analyzer load tests

3 participants

@coveralls@jackfirth