-
Notifications
You must be signed in to change notification settings - Fork 1
test: Тест проверки выделяемой памяти пустого бенчмарка #20
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
Conversation
WalkthroughA test procedure was renamed for clarity, and the method for accessing allocated memory in benchmark results was updated. Additionally, a new test procedure was added to verify memory allocation for an empty benchmark, including assertions on memory usage. Another test procedure was modified to check memory allocation via run statistics instead of report tables. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant Benchmark
participant Result
TestRunner->>Benchmark: Run benchmark with memory monitoring enabled
Benchmark-->>Result: Return run results including memory statistics
TestRunner->>Result: Access allocated memory in run statistics
TestRunner->>TestRunner: Assert memory allocation is within expected bounds
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (14)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/ТестыЗамеров.os
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: sonar / test
- GitHub Check: build (ubuntu-latest, stable)
- GitHub Check: build (windows-latest, dev)
- GitHub Check: build (windows-latest, stable)
- GitHub Check: build (macos-latest, dev)
- GitHub Check: build (ubuntu-latest, dev)
- GitHub Check: build (macos-latest, stable)
- GitHub Check: sonar / test
- GitHub Check: build (macos-latest, stable)
- GitHub Check: build (ubuntu-latest, stable)
- GitHub Check: build (macos-latest, dev)
- GitHub Check: build (windows-latest, stable)
- GitHub Check: build (ubuntu-latest, dev)
- GitHub Check: build (windows-latest, dev)
🔇 Additional comments (2)
tests/ТестыЗамеров.os (2)
8-8
: Good naming improvement!The procedure rename makes the test intent more explicit and follows better naming conventions for test methods.
27-43
:⚠️ Potential issueFix syntax error and consider narrowing assertion range.
The new test procedure for empty benchmark memory allocation is well-structured, but has issues:
- Syntax Error: Missing semicolon after line 32
- Assertion Range: The range 1-600 bytes seems quite wide for an empty benchmark test
Apply this diff to fix the syntax error:
- Конфигурация = Новый КонфигурацияБенчмарков(Тип) + Конфигурация = Новый КонфигурацияБенчмарков(Тип);Consider narrowing the assertion range for more precise testing. For an empty benchmark, the memory allocation should be more predictable:
- Ожидаем.Что(ВыделеноБайт).Между(1, 600); + Ожидаем.Что(ВыделеноБайт).Между(1, 100);Likely an incorrect or invalid review comment.
ВыделеноБайт = Результат.Отчет.Таблица[0][КолонкиОтчетаБенчмарков.ВыделяемаяПамять]; | ||
|
||
|
||
ВыделеноБайт = Результат.РезультатыЗапусков[0].Статистика.ВыделяемаяПамять; |
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.
💡 Verification agent
🧩 Analysis chain
API update looks good - verify consistency across codebase.
The new way of accessing allocated memory statistics through РезультатыЗапусков[0].Статистика.ВыделяемаяПамять
is more direct and clear than the previous table-based access.
Ensure that all other occurrences of the old API pattern are updated consistently:
🏁 Script executed:
#!/bin/bash
# Description: Search for any remaining usage of the old memory access pattern
# Expected: No occurrences of the old pattern should remain
# Search for old pattern usage
rg "Отчет\.Таблица.*ВыделяемаяПамять" --type os
rg "КолонкиОтчетаБенчмарков\.ВыделяемаяПамять" --type os
Length of output: 162
🏁 Script executed:
#!/usr/bin/env bash
# Re-run search for old memory access pattern without file-type restriction
# Expected: no matches if all old API calls have been replaced
rg "Отчет\.Таблица.*ВыделяемаяПамять"
rg "КолонкиОтчетаБенчмарков\.ВыделяемаяПамять"
Length of output: 354
Fix remaining old memory API references
The search uncovered two places still using the old Отчет.Таблица…ВыделяемаяПамять
pattern. Please update these to use the new РезультатыЗапусков[0].Статистика.ВыделяемаяПамять
API (or remove them if no longer needed):
• tests/ТестыКонфигурации.os
// before
Ожидаем.Что(Результат.Отчет.Таблица[0]["ВыделяемаяПамять"]).Заполнено();
// after
Ожидаем.Что(Результат.РезультатыЗапусков[0].Статистика.ВыделяемаяПамять).Заполнено();
• src/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковВыделяемаяПамять.os
// before
Колонка = Новый КолонкаОтчетаБенчмарков(КолонкиОтчетаБенчмарков.ВыделяемаяПамять);
// after (example)
Колонка = Новый КолонкаСтатистики(Статистика.ВыделяемаяПамять);
(Or remove this column if the table-based report is deprecated.)
🤖 Prompt for AI Agents
In tests/ТестыЗамеров.os at line 20 and related files tests/ТестыКонфигурации.os
and
src/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковВыделяемаяПамять.os,
update all remaining old API references accessing allocated memory statistics
via Отчет.Таблица or КолонкиОтчетаБенчмарков.ВыделяемаяПамять to use the new
direct API РезультатыЗапусков[0].Статистика.ВыделяемаяПамять. Replace
expressions like Результат.Отчет.Таблица[0]["ВыделяемаяПамять"] with
Результат.РезультатыЗапусков[0].Статистика.ВыделяемаяПамять, and refactor or
remove column definitions relying on the old table-based pattern accordingly.
|
Summary by CodeRabbit