Conversation
Co-authored-by: jackfirth <8175575+jackfirth@users.noreply.github.com>
There was a problem hiding this comment.
Resyntax analyzed 1 file in this pull request and found no issues.
There was a problem hiding this comment.
Put all of this inside of a (module+ test ...) submodule.
There was a problem hiding this comment.
Done in 1ab6849. All code is now inside (module+ test ...).
test/analyzer-load-test.rkt
Outdated
| (define analysis | ||
| (source-analyze large-test-source | ||
| #:analyzers (list variable-mutability-analyzer) | ||
| #:timeout-ms timeout-ms)) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
test/analyzer-load-test.rkt
Outdated
| (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))) |
There was a problem hiding this comment.
Don't bother with this load test.
- 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>
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)expansion-analyzedirectly withtime-applyto measure actual analyzer performancesource-expandand reuses it for all tests(module+ test ...)submoduleEach test measures elapsed time and fails with diagnostic message if timeout exceeded:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.