Skip to content

test: resolve issues#11

Merged
AsakuraMizu merged 7 commits intoStarry-OS:mainfrom
HoshimiP:adjust-tests
Dec 20, 2025
Merged

test: resolve issues#11
AsakuraMizu merged 7 commits intoStarry-OS:mainfrom
HoshimiP:adjust-tests

Conversation

@HoshimiP
Copy link
Copy Markdown
Contributor

@HoshimiP HoshimiP commented Dec 17, 2025

Supplement to #10

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR consolidates multi-threaded signal handling tests by removing the separate tests/multithread.rs file and integrating its tests into tests/api_thread.rs. The changes also introduce critical bug fixes for integer overflow handling in virtual memory operations and improve test reliability by fixing race condition detection logic.

Key changes:

  • Added proper overflow protection in VmIo implementation using checked_sub and checked_add operations
  • Fixed a bug in the thread_handler test that could produce false positives by using bit flags instead of counters to track distinct signal deliveries
  • Consolidated test utilities and test cases from a separate file into the main thread API test file

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/multithread.rs Removed separate multithreading test file; tests moved to api_thread.rs
tests/api_thread.rs Added overflow-safe arithmetic in VmIo methods, integrated multithreading tests with improved signal delivery tracking logic

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


static POOL: LazyLock<Mutex<Box<[u8]>>> = LazyLock::new(|| {
let size = 0x0100_0000; // 1 MiB
let size = 0x0100_0000;
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

The comment indicating the size unit "1 MiB" has been removed. The value 0x0100_0000 is actually 16 MiB (16,777,216 bytes), not 1 MiB (1,048,576 bytes). While removing the incorrect comment avoids misleading information, consider adding a correct comment such as "16 MiB" for clarity.

Suggested change
let size = 0x0100_0000;
let size = 0x0100_0000; // 16 MiB

Copilot uses AI. Check for mistakes.
@AsakuraMizu
Copy link
Copy Markdown
Contributor

What's your idea on #10 (comment)? I think it is good to reduce code duplication.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

AsakuraMizu and others added 2 commits December 20, 2025 22:43
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@AsakuraMizu AsakuraMizu merged commit bf76985 into Starry-OS:main Dec 20, 2025
12 checks passed
@HoshimiP
Copy link
Copy Markdown
Contributor Author

What's your idea on #10 (comment)? I think it is good to reduce code duplication.

I understand...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants