-
Notifications
You must be signed in to change notification settings - Fork 120
Improve disk speed test #3743
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.
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
Improve disk speed test #3743
Conversation
Reviewer's GuideThis PR replaces the single 1GiB disk speed test with a multi-size, streaming test that collects multiple DiskSpeedTestPoint results, updates the Vuex store and API to support streaming NDJSON responses, and enhances the Disk.vue UI to show progressive test status, averages, and a graph suited for low-speed SD cards. Sequence diagram for multi-size streaming disk speed testsequenceDiagram
actor User
participant DiskView
participant DiskStore
participant back_axios
participant DiskAPI as DiskUsageAPI
participant Generator as multi_size_speed_test_generator
participant Disktest as disktest_binary
User->>DiskView: Click Run Speed Test
DiskView->>DiskStore: runMultiSizeSpeedTest()
DiskStore->>DiskStore: setSpeedTesting(true)
DiskStore->>DiskStore: setSpeedError(null), setSpeedResults([])
DiskStore->>DiskStore: setSpeedTestProgress(Starting tests...)
DiskStore->>back_axios: GET /speed/stream (onDownloadProgress)
back_axios->>DiskAPI: GET /speed/stream
DiskAPI->>Generator: multi_size_speed_test_generator()
loop For each size_mb in [10,50,100,200]
Generator->>Disktest: run_single_speed_test(size_mb * 1024 * 1024)
Disktest-->>Generator: DiskSpeedResult(write_speed_mbps, read_speed_mbps)
Generator->>Generator: Build DiskSpeedTestPoint(size_mb, write_speed, read_speed)
Generator-->>DiskAPI: JSON line for DiskSpeedTestPoint
DiskAPI-->>back_axios: NDJSON chunk
back_axios-->>DiskStore: onDownloadProgress(response)
DiskStore->>DiskStore: parseStreamingResponse(response)
DiskStore->>DiskStore: addSpeedResult(point)
DiskStore->>DiskStore: setSpeedTestProgress(Tested X MB)
end
DiskAPI-->>back_axios: Stream completed
back_axios-->>DiskStore: Promise resolved
DiskStore->>DiskStore: setSpeedTestProgress(Test complete)
DiskStore->>DiskStore: setSpeedTesting(false)
DiskStore-->>DiskView: speedResults, speedTestProgress, speedTesting
DiskView-->>User: Show progress, averages, and graph
Updated class diagram for disk speed test models and storeclassDiagram
class DiskSpeedResult_py {
+float~Optional~ write_speed_mbps
+float~Optional~ read_speed_mbps
+bool success
+str~Optional~ error
}
class DiskSpeedTestPoint_py {
+int size_mb
+float~Optional~ write_speed
+float~Optional~ read_speed
}
class DiskSpeedResult_ts {
+number~Optional~ write_speed_mbps
+number~Optional~ read_speed_mbps
+bool success
+string~Optional~ error
}
class DiskSpeedTestPoint_ts {
+number size_mb
+number~Optional~ write_speed
+number~Optional~ read_speed
}
class DiskStore {
-string API_URL
-DiskSpeedResult_ts~Optional~ speedResult
-DiskSpeedTestPoint_ts[] speedResults
-bool speedTesting
-string speedTestProgress
-string~Optional~ speedError
+setAPIUrl(value string) void
+setSpeedResult(value DiskSpeedResult_ts~Optional~) void
+setSpeedResults(value DiskSpeedTestPoint_ts[]) void
+addSpeedResult(point DiskSpeedTestPoint_ts) void
+setSpeedTesting(value bool) void
+setSpeedTestProgress(value string) void
+setSpeedError(message string~Optional~) void
+fetchUsage() Promise~void~
+runSpeedTest(sizeBytes number) Promise~void~
+runMultiSizeSpeedTest() Promise~void~
}
class DiskView {
+int activeTab
+bool loaded
+bool loading
+string~Optional~ error
+DiskSpeedTestPoint_ts[] speedResults
+bool speedTesting
+string speedTestProgress
+string~Optional~ speedError
+bool hasResults
+string avgWriteSpeed()
+string avgReadSpeed()
+string state()
+mounted() void
+runSpeedTest() Promise~void~
}
class DiskRouter {
+disk_speed(size_bytes int) DiskSpeedResult_py
+run_single_speed_test(size_bytes int) DiskSpeedResult_py
+multi_size_speed_test_generator() AsyncGenerator~str, None~
+disk_speed_stream() StreamingResponse
}
DiskRouter --> DiskSpeedResult_py
DiskRouter --> DiskSpeedTestPoint_py
DiskRouter --> DiskSpeedResult_ts
DiskRouter --> DiskSpeedTestPoint_ts
DiskStore --> DiskSpeedResult_ts
DiskStore --> DiskSpeedTestPoint_ts
DiskView --> DiskStore
DiskView --> DiskSpeedTestPoint_ts
DiskView --> DiskSpeedGraph_ts
class DiskSpeedGraph_ts {
+DiskSpeedTestPoint_ts[] data
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 1 issue, and left some high level feedback:
- The streaming speed test endpoint currently yields
json.dumps(point.dict())without a newline, which breaks theapplication/x-ndjsoncontract and can make client-side parsing brittle; consider appending a\nto each yielded JSON object so each record is clearly delimited. - The frontend assumes exactly 4 test points (e.g.,
speedResults.length * 100 / 4,{{ speedResults.length }}/4, andthis.speedResults.length === 4), which is tightly coupled totest_sizes_mb = [10, 50, 100, 200]; consider deriving the expected count from the backend or a shared constant so future changes to test sizes don't require updating multiple magic numbers in the UI.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The streaming speed test endpoint currently yields `json.dumps(point.dict())` without a newline, which breaks the `application/x-ndjson` contract and can make client-side parsing brittle; consider appending a `\n` to each yielded JSON object so each record is clearly delimited.
- The frontend assumes exactly 4 test points (e.g., `speedResults.length * 100 / 4`, `{{ speedResults.length }}/4`, and `this.speedResults.length === 4`), which is tightly coupled to `test_sizes_mb = [10, 50, 100, 200]`; consider deriving the expected count from the backend or a shared constant so future changes to test sizes don't require updating multiple magic numbers in the UI.
## Individual Comments
### Comment 1
<location> `core/services/disk_usage/main.py:434-439` </location>
<code_context>
+ size_bytes = size_mb * 1024 * 1024
+ result = await run_single_speed_test(size_bytes)
+
+ point = DiskSpeedTestPoint(
+ size_mb=size_mb,
+ write_speed=result.write_speed_mbps,
+ read_speed=result.read_speed_mbps,
+ )
+ yield json.dumps(point.dict())
+
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Streamed NDJSON should ensure records are newline-delimited to be valid and easily parsable.
This endpoint returns `application/x-ndjson`, but `multi_size_speed_test_generator` yields JSON strings without a newline terminator. NDJSON typically requires one JSON object per line (`
`-terminated). Unless `streamer` is already handling this, please either append `"\n"` to each yielded string or document that `streamer` guarantees newline-delimited output; otherwise consumers like `parseStreamingResponse` may not be able to split records correctly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
4dc701e to
1cd7ac3
Compare
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
1cd7ac3 to
50e3f10
Compare
There are sd cards running at 2MB read / write speeds.
This test improve the test for low speed cards
Summary by Sourcery
Enhance the disk speed test to run multiple streaming measurements and present aggregated results with improved UI feedback.
New Features:
Enhancements: