Skip to content

Fix assembly bugs in EMS integration tests#1449

Merged
maximilien-noal merged 2 commits into
copilot/add-ems-unit-testsfrom
copilot/sub-pr-1448
Oct 29, 2025
Merged

Fix assembly bugs in EMS integration tests#1449
maximilien-noal merged 2 commits into
copilot/add-ems-unit-testsfrom
copilot/sub-pr-1448

Conversation

Copilot AI commented Oct 29, 2025

Copy link
Copy Markdown
Contributor

Three EMS integration tests were failing due to incorrect x86 assembly encoding in the test programs. The EMS implementation itself is correct.

Fixes

  • Device name detection test: Missing DS segment setup and incorrect offset calculation

    • Added push cs; pop ds to set DS=CS before DOS file open call
    • Corrected device name offset from 0x20 to 0x1E (accounting for COM load address)
  • Memory access tests: Missing ES segment override prefixes on write instructions

    • Added 0x26 prefix to mov byte [es:0], imm instructions (3 occurrences)
    • Adjusted conditional jump offsets to account for added bytes

Example

Before (writes to DS:0 instead of ES:0):

0xC6, 0x06, 0x00, 0x00, 0x42  // mov byte [0], 42h - uses DS by default

After (correctly writes to ES:0):

0x26, 0xC6, 0x06, 0x00, 0x00, 0x42  // mov byte [es:0], 42h - ES prefix

All 17 integration tests and 47 unit tests now pass.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>

Copilot AI commented Oct 29, 2025

Copy link
Copy Markdown
Contributor Author

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • av-build-tel-api-v1.avaloniaui.net
    • Triggering command: dotnet exec --runtimeconfig /home/REDACTED/.nuget/packages/avalonia.buildservices/11.3.1/tools/netstandard2.0/runtimeconfig.json /home/REDACTED/.nuget/packages/avalonia.buildservices/11.3.1/tools/netstandard2.0/Avalonia.BuildServices.Collector.dll (dns block)
    • Triggering command: dotnet test tests/Spice86.Tests/Spice86.Tests.csproj --filter FullyQualifiedName~EmsIntegrationTests --logger console;verbosity=detailed (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Fix remaining EMS integration tests to pass Fix assembly bugs in EMS integration tests Oct 29, 2025
Copilot AI requested a review from maximilien-noal October 29, 2025 07:31
@maximilien-noal maximilien-noal marked this pull request as ready for review October 29, 2025 08:13
@maximilien-noal maximilien-noal merged commit a31ab04 into copilot/add-ems-unit-tests Oct 29, 2025
9 checks passed
maximilien-noal added a commit that referenced this pull request Oct 29, 2025
* Initial plan

* Add EMS unit and integration test structure with initial tests

Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>

* Fix EMS tests to call methods directly, 16 unit tests and 12 integration tests passing

Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>

* Fix EMS unit tests - 33 out of 48 passing, all integration tests completed

Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>

* Fix EMS unit tests - remove leftover state setup, now 40/48 passing

Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>

* Fix missing method calls in EMS tests - now 42/48 unit, 13/17 integration passing

Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>

* Fix GetExpandedMemoryHardwareInformation test - now 43/48 unit tests passing

Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>

* Changes before error encountered

Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>

* fix: EMS Restore Page Map is dependant on Save Pag Map being called first

* Fix assembly bugs in EMS integration tests (#1449)

* Initial plan

* Fix EMS integration test assembly bugs - all 17 tests now pass

Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>

* chore: typo

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: maximilien-noal <1087524+maximilien-noal@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@maximilien-noal maximilien-noal deleted the copilot/sub-pr-1448 branch November 14, 2025 06:02
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.

2 participants