Skip to content
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

Add test for the reboot endpoint #635

Merged
merged 1 commit into from
Jun 21, 2024
Merged

Add test for the reboot endpoint #635

merged 1 commit into from
Jun 21, 2024

Conversation

olethanh
Copy link
Collaborator

No description provided.

Copy link

This PR introduces a new file and a new test case. The new file, test_operator.py, adds a new module for testing VM execution engine. The test case test_reboot_ok tests the reboot functionality of the VM.

The test case uses a mock for authentication and a mock for the VM pool, which is a potential source of complexity for the reviewer. The test case also uses aiohttp for creating a test client, which might be unfamiliar to the reviewer.

Highlights:

  • test_reboot_ok: This test case uses aiohttp to create a test client, which might be unfamiliar to the reviewer. The test case also uses mocks for authentication and the VM pool, which could potentially introduce bugs if not handled correctly.

  • test_operator.py: This new file introduces a new module for testing the VM execution engine, which could potentially introduce new bugs if not handled correctly.

Consideration for the codebase:

  • The introduction of a new file and test case could potentially introduce new bugs if not handled correctly.
  • The use of mocks for authentication and the VM pool could potentially introduce bugs if not handled correctly.
  • The use of aiohttp to create a test client might be unfamiliar to the reviewer and could potentially introduce bugs if not handled correctly.

This PR seems to have moderate complexity, but it could require a deeper understanding of the project architecture and the potential for introducing new bugs.

System Prompt

Your name is Code Review Categorizer 2.0 (CRC2). You rate Github PRs for review complexity based on a more nuanced understanding of codebase architecture, potential for introducing new bugs, and complexity of the review process.

The CRC2 is programmed to analyze GitHub Pull Requests (PRs) and categorize them into four levels of review complexity: 'BLUE', 'GREEN', 'YELLOW', or 'RED'. It assesses the PR based on various indicators for each category. For 'BLUE', indicators may include small changes, simple refactoring, or documentation updates that are unlikely to introduce bugs. 'GREEN' may involve changes that are likely to introduce new features or improvements to existing features. 'YELLOW' could involve changes that could potentially introduce bugs or have moderate impact on the codebase but require a deep understanding of the project architecture and could be more challenging to review. 'RED' is assigned to PRs with extensive changes that have a high potential for introducing bugs or that require deep understanding of the project architecture. Only the most experienced developers should review 'RED' PRs, and the label should be used sparingly, if possible.

The user will provide additional information and rules, in this format:

--- ---

Rules: <users' rules>

Diff:

The CRC2 provides a clear explanation for the categorization, considering the potential impact on the codebase and review process. The user may supply specific rules to fine-tune the categorization process. The CRC2's responses are designed to be parsed by machines for integration into other systems, therefore its response starts with a line of the exact category it is rated, and followed by multiple lines of summary and highlighting relevant parts of the diff using markdown.

User Message

--- aleph-im/aleph-vm

Rules: No additional rules provided

Diff:
diff –-git a/tests/supervisor/views/test_operator.py b/tests/supervisor/views/test_operator.py
new file mode 100644
index 00000000..3e50fe53
--- /dev/null
+++ b/tests/supervisor/views/test_operator.py
@@ -0,0 +1,38 @@
+import pytest
+
+from aleph.vm.orchestrator.supervisor import setup_webapp
+
+
[email protected]
+async def test_reboot_ok(aiohttp_client, mocker):

  • mock_address = "mock_address"
  • mock_hash = "fake_vm_fake_vm_fake_vm_fake_vm_fake_vm_fake_vm_fake_vm_fake_vm_"
  • mocker.patch(
  •    "aleph.vm.orchestrator.views.authentication.authenticate_jwk",
    
  •    return_value=mock_address,
    
  • )
  • class FakeVmPool:
  •    executions = {
    
  •        mock_hash: mocker.Mock(
    
  •            vm_hash=mock_hash,
    
  •            message=mocker.Mock(address=mock_address),
    
  •            is_confidential=False,
    
  •            is_running=True,
    
  •        ),
    
  •    }
    
  •    systemd_manager = mocker.Mock(restart=mocker.Mock())
    
  • app = setup_webapp()
  • pool = FakeVmPool()
  • app["vm_pool"] = pool
  • app["pubsub"] = FakeVmPool()
  • client = await aiohttp_client(app)
  • response = await client.post(
  •    f"/control/machine/{mock_hash}/reboot",
    
  • )
  • assert response.status == 200
  • assert (
  •    await response.text()
    
  •    == "Rebooted VM with ref fake_vm_fake_vm_fake_vm_fake_vm_fake_vm_fake_vm_fake_vm_fake_vm_"
    
  • )
  • assert pool.systemd_manager.restart.call_count == 1

Assistant

Category: YELLOW

This PR introduces a new file and a new test case. The new file, test_operator.py, adds a new module for testing VM execution engine. The test case test_reboot_ok tests the reboot functionality of the VM.

The test case uses a mock for authentication and a mock for the VM pool, which is a potential source of complexity for the reviewer. The test case also uses aiohttp for creating a test client, which might be unfamiliar to the reviewer.

Highlights:

  • test_reboot_ok: This test case uses aiohttp to create a test client, which might be unfamiliar to the reviewer. The test case also uses mocks for authentication and the VM pool, which could potentially introduce bugs if not handled correctly.

  • test_operator.py: This new file introduces a new module for testing the VM execution engine, which could potentially introduce new bugs if not handled correctly.

Consideration for the codebase:

  • The introduction of a new file and test case could potentially introduce new bugs if not handled correctly.
  • The use of mocks for authentication and the VM pool could potentially introduce bugs if not handled correctly.
  • The use of aiohttp to create a test client might be unfamiliar to the reviewer and could potentially introduce bugs if not handled correctly.

This PR seems to have a high level of complexity due to the introduction of a new file and test case, and the use of mocks and aiohttp. It also requires a deeper understanding of the project architecture and the potential for introducing new bugs. Therefore, it is assigned the 'YELLOW' category. Only the most experienced developers should review this PR, and the label should be used sparingly, if possible.

@github-actions github-actions bot added the RED This PR is complex and may require more time to review. label Jun 17, 2024
Copy link
Member

@nesitor nesitor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe move that test to already existing test_operator.py file? Also, you can move the entire file to that folder.

@olethanh
Copy link
Collaborator Author

sorry I didn't see it beceause it's in the dev confidential branch and this is based on main.

@olethanh
Copy link
Collaborator Author

Will merge this in main and then update dev-confidential branch to have both file merged.

@olethanh olethanh merged commit 4e350d6 into main Jun 21, 2024
25 checks passed
@Psycojoker Psycojoker deleted the ol_test_operator_reboot branch July 24, 2024 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RED This PR is complex and may require more time to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants