test(exec): add ui-queue default limits integration test (MILAB-5798)#1491
test(exec): add ui-queue default limits integration test (MILAB-5798)#1491
Conversation
Add `uiQueueUsesForBatchSetup` to feats.lib.tengo and update the fallback runner selection logic in exec/limits.lib.tengo. When the flag is false, the ui-tasks queue uses RunCommand/batch instead of the default RunCommand/executor, enabling batch execution for UI tasks.
Verifies that commands run in the ui-tasks queue without explicit cpu/ram receive the correct defaults: 1 CPU core and 100 MiB RAM.
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new integration test to validate the default resource limits for the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds an integration test to verify the default resource limits for the ui-tasks queue. The changes include a new feature flag to control the runner type for this queue, a new test template, and the corresponding test case. The implementation is correct, but I have a couple of suggestions to improve the new test code. First, the test case in exec.test.ts uses magic numbers for the expected limits, which should be extracted into constants for better readability and maintainability. Second, the new test template echo_ui_queue_limits.tpl.tengo executes two separate commands to fetch the CPU and RAM limits, which is inefficient. It can be refactored to use a single command execution.
| tplTest.concurrent("ui-queue-default-limits", async ({ helper, expect }) => { | ||
| const result = await helper.renderTemplate( | ||
| false, | ||
| "exec.run.echo_ui_queue_limits", | ||
| ["cpu", "ram"], | ||
| (_tx) => ({}), | ||
| ); | ||
|
|
||
| const cpu = await result.computeOutput("cpu", (a) => a?.getDataAsString()).awaitStableValue(); | ||
| const ram = await result.computeOutput("ram", (a) => a?.getDataAsString()).awaitStableValue(); | ||
|
|
||
| expect(Number(cpu)).eq(1); // QueueUITasksDefaultTaskCores | ||
| expect(Number(ram)).eq(100 * 1024 * 1024); // QueueUITasksDefaultTaskRAM = 100 MiB | ||
| }); |
There was a problem hiding this comment.
The test uses magic numbers for the expected default CPU cores and RAM. It's better to define these as constants at the beginning of the test case. This improves readability and maintainability, as the values are documented in one place and can be easily updated if the defaults change.
tplTest.concurrent("ui-queue-default-limits", async ({ helper, expect }) => {
const UI_QUEUE_DEFAULT_CPU_CORES = 1;
const UI_QUEUE_DEFAULT_RAM_BYTES = 100 * 1024 * 1024;
const result = await helper.renderTemplate(
false,
"exec.run.echo_ui_queue_limits",
["cpu", "ram"],
(_tx) => ({}),
);
const cpu = await result.computeOutput("cpu", (a) => a?.getDataAsString()).awaitStableValue();
const ram = await result.computeOutput("ram", (a) => a?.getDataAsString()).awaitStableValue();
expect(Number(cpu)).eq(UI_QUEUE_DEFAULT_CPU_CORES);
expect(Number(ram)).eq(UI_QUEUE_DEFAULT_RAM_BYTES);
});
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1491 +/- ##
==========================================
- Coverage 54.71% 54.68% -0.03%
==========================================
Files 242 242
Lines 13576 13576
Branches 2783 2783
==========================================
- Hits 7428 7424 -4
- Misses 5219 5224 +5
+ Partials 929 928 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Replace bash command with hello-world software asset for echoing system limits in the ui-queue integration test.
Replace the single ui-tasks queue test with a parameterized test covering all four queues (ui-tasks, heavy, medium, light). Use a single shared template that accepts a queue name input via inQueue(), asserting cpu/ram are > 0 and <= the queue's configured default.
3 participants
Summary
echo_ui_queue_limits.tpl.tengotemplate that runs two bash commands in theui-tasksqueue (no explicit cpu/ram), echoing{system.cpu}and{system.ram.bytes}ui-queue-default-limitstest inexec.test.tsverifying the resolved values equal the queue defaults: 1 CPU core and 100 MiB RAMTest plan
pnpm build && pnpm test -- -t "ui-queue-default-limits"passes with cpu=1, ram=104857600