test(agent): add unit tests for MockVmMgr and ConsoleProxyResource#25
test(agent): add unit tests for MockVmMgr and ConsoleProxyResource#25devin-ai-integration[bot] wants to merge 1 commit into
Conversation
Add comprehensive unit tests for two untested classes in the agent module: - MockVmMgrTest: 31 tests covering VM lifecycle (start, stop, reboot, migrate, cleanup), VNC port allocation/exhaustion, memory accounting, host resource queries, and createVmFromSpec. - ConsoleProxyResourceTest: 16 tests covering command dispatch (Ready, CheckHealth, unsupported), configure() parameter parsing (premium port, custom port, default port, eth1/eth2, public IP), initialize(), getCurrentStatus(), and getDefaultScriptsDir. Co-Authored-By: hitomi.sawamura@gmail.com <hitomi.sawamura@gmail.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| @Test | ||
| public void testRebootVm() { | ||
| vmMgr.startVM("vm1", "vnet1", "10.0.0.1", "8.8.8.8", | ||
| "192.168.1.10", "aa:bb:cc:dd:ee:01", "255.255.255.0", | ||
| "10.0.0.10", "aa:bb:cc:dd:ee:02", "255.255.255.0", | ||
| 2, 50, 1024, "/local/path", "password"); | ||
|
|
||
| vmMgr.stopVM("vm1", false); | ||
| vmMgr.rebootVM("vm1"); | ||
|
|
||
| Assert.assertEquals(State.Running, vmMgr.getVm("vm1").getState()); | ||
| } |
There was a problem hiding this comment.
🚩 testRebootVm tests stop-then-reboot, which leaves VNC port freed
This test calls stopVM (which frees the VNC port via MockVmMgr.java:96) then rebootVM (which only sets state to Running at MockVmMgr.java:110 without re-allocating a VNC port). After the sequence, the VM is in Running state but its VNC port has been freed from the bitmask, meaning another VM could be allocated the same port. The test correctly validates the current implementation behavior, but this reveals a potential pre-existing bug in MockVmMgr.rebootVM() — it doesn't restore the VNC port allocation. In practice this mock class may never encounter this scenario, but it's worth noting.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good observation. The test intentionally validates the current rebootVM() behavior (state-only transition without VNC port restoration). This is indeed a pre-existing quirk in MockVmMgr — stopVM frees the port but rebootVM doesn't re-allocate it. Since this is a mock/test utility class and the typical use case is rebootVM on a running VM (not stop→reboot), it's low risk. The test documents the current contract as-is.
| public void setUp() throws ConfigurationException { | ||
| Map<String, Object> params = new HashMap<>(); | ||
| params.put("name", "testProxy"); | ||
| params.put("zone", "1"); | ||
| params.put("pod", "1"); | ||
| params.put("guid", "test-guid"); | ||
| params.put("proxy_vm", "100"); | ||
| params.put("consoleproxy.httpListenPort", "8080"); | ||
| resource.configure("testProxy", params); | ||
| } |
There was a problem hiding this comment.
🚩 ConsoleProxyResource tests depend on host network interfaces being discoverable
The setUp() calls resource.configure("testProxy", params) on a @Spy, which invokes the real ConsoleProxyResource.configure → super.configure (ServerResourceBase.java:73-91). The superclass attempts to find network interfaces via defineResourceNetworkInterfaces and tryToAutoDiscoverResourcePrivateNetworkInterface. In environments without standard NICs (e.g., certain CI containers), tryToAutoDiscoverResourcePrivateNetworkInterface at ServerResourceBase.java:132 throws ConfigurationException. The test would then fail with a ConfigurationException in setUp(). This is environment-dependent and may or may not be an issue depending on CI setup.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged. In practice, the ServerResourceBase.configure() calls defineResourceNetworkInterfaces which only throws ConfigurationException when private.network.device is explicitly set but unresolvable. Since our test params don't set private.network.device (no eth1ip), the auto-discovery path gracefully handles missing NICs by logging a warning. This test passes in GitHub Actions CI as confirmed by the passing checks.
There was a problem hiding this comment.
🚩 Default memory value 16000L in MockVmMgr is suspiciously low for a 'bytes' field
The testConfigureWithDefaults test validates that getHostTotalMemory() returns 16000L when no memory param is provided. This matches the source code default at MockVmMgr.java:194, but the method comment says "total memory in bytes". 16000 bytes (≈16 KB) is unrealistically small for a host's total memory — it's likely a pre-existing issue where the default should be something like 16000L * 1024 * 1024 (16 GB). The test correctly validates current behavior, but the underlying default may be wrong.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Correct — this test documents the pre-existing default value. The 16000L default in MockVmMgr.getHostTotalMemory() appears to be a placeholder that's always overridden by the configured "memory" param in production use. A unit mismatch fix in the source would be a separate concern outside this test PR's scope.
Description
This PR adds comprehensive unit tests for two previously untested classes in the
agentmodule:MockVmMgrTest (31 tests):
createVmFromSpec: normal creation, out-of-memory exception, idempotencyConsoleProxyResourceTest (16 tests):
executeRequest: ReadyCommand, CheckHealthCommand, unsupported commandsconfigure()parameter parsing: premium port (443), custom httpListenPort, default port (80), eth1/eth2 network device mapping, public IPinitialize(),getCurrentStatus(),getRunLevel(),getConfigParams()Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
How Has This Been Tested?
All 47 tests pass (31 + 16). Tests use JUnit 4 + Mockito consistent with the existing test style in the module.</pr_template>
How did you try to break this feature and the system with this change?
Tests only — no production code modified. Verified edge cases like VNC port exhaustion (all 64 ports), out-of-memory conditions, and null/missing VM lookups.</pr_template>
Link to Devin session: https://app.devin.ai/sessions/8be1899eb2834f11a3e166b1bb2783c0
Requested by: @hitomimimi