Refactor CPU instruction tests and enhance development setup #53

hspaans · 2025-10-13T00:45:18Z

Refactor tests for CPU instructions and memory operations to improve clarity and maintainability. Configure the development environment with updated linting tools and documentation tasks. Remove obsolete files and streamline package installation settings.

…memory operations - Removed redundant imports and cleaned up test functions for LDA, LDX, LDY, STA, STX, and STY instructions. - Consolidated test cases to improve readability and maintainability. - Updated memory test cases to directly use the Memory class from m6502. - Ensured all tests maintain functionality while enhancing clarity and structure.

…tings, and update documentation build tasks

Copilot

Pull Request Overview

This PR refactors the CPU instruction test suite and modernizes the development environment configuration to improve maintainability and streamline the development workflow.

Consolidates individual CPU instruction test files into a single comprehensive test file
Migrates from setup.cfg/tox.ini to modern pyproject.toml and tox.toml configuration files
Reorganizes code structure with proper src/ layout and removes obsolete files

Reviewed Changes

Copilot reviewed 22 out of 24 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
tox.toml	New tox configuration with modern format and comprehensive testing environments
tests/test_memory.py	Updated import statements to use direct Memory import
tests/test_cpu_ins_*.py	Removed individual instruction test files (content consolidated)
tests/test_cpu.py	Expanded with comprehensive CPU instruction tests covering all addressing modes
src/m6502/	New source directory structure with processor and memory modules
pyproject.toml	New project configuration with modern Python packaging standards
setup.cfg	Removed obsolete configuration file
.flake8	New flake8 configuration file

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-13T00:46:46Z

tox.toml

+dependency_groups = ["test"]
+pass_env = ["PYTEST_*", "SSL_CERT_FILE"]
+set_env.COVERAGE_FILE = { replace = "env", name = "COVERAGE_FILE", default = "{work_dir}{/}.coverage.{env_name}" }
+set_env.COVERAGE_FILECOVERAGE_PROCESS_START = "{tox_root}{/}pyproject.toml"


Missing underscore in environment variable name. Should be 'COVERAGE_FILE_COVERAGE_PROCESS_START' instead of 'COVERAGE_FILECOVERAGE_PROCESS_START'.

Suggested change

set_env.COVERAGE_FILECOVERAGE_PROCESS_START = "{tox_root}{/}pyproject.toml"

set_env.COVERAGE_FILE_COVERAGE_PROCESS_START = "{tox_root}{/}pyproject.toml"

tests/test_memory.py

tests/test_cpu.py

Copilot · 2025-10-13T00:46:47Z

tests/test_cpu.py

+    after the opcode. The memory location is a two-byte address that can
+


Incomplete documentation. The description is cut off mid-sentence and doesn't explain the addressing mode fully.

Suggested change

after the opcode. The memory location is a two-byte address that can

calculated by adding the X register to the two-byte address specified

after the opcode. The base address can range from 0x0000 to 0xFFFF.

Copilot · 2025-10-13T00:46:48Z

src/m6502/processor.py

+        """
+        while (self.cycles < cycles) or (cycles == 0):
+            opcode = self._fetch_byte()
+            eval("self._ins_" + self._OPCODES[opcode] + "_" + self._ADDRESSING[opcode] + "()")  # noqa: PLW123


Using eval() is a security risk and should be replaced with a safer alternative like getattr() or a dispatch dictionary.

Suggested change

eval("self._ins_" + self._OPCODES[opcode] + "_" + self._ADDRESSING[opcode] + "()") # noqa: PLW123

method_name = "_ins_" + self._OPCODES[opcode] + "_" + self._ADDRESSING[opcode]

getattr(self, method_name)()

src/m6502/processor.py

Copilot · 2025-10-13T00:46:49Z

src/m6502/memory.py

+        :rtype: int
+        :raises ValueError: Memory address is not valid
+        """
+        if 0x0000 < address > self.size:


Invalid comparison logic using chained comparison operators. This will always evaluate to False. Should use 'address < 0x0000 or address > self.size' instead.

Copilot · 2025-10-13T00:46:49Z

src/m6502/memory.py

+        :raises ValueError: Memory address is not valid
+        :raises ValueError: Size of value exceed byte size
+        """
+        if 0x0000 < address > self.size:


Invalid comparison logic using chained comparison operators. This will always evaluate to False. Should use 'address < 0x0000 or address > self.size' instead.

…ion, and enhance linting configuration

- Added blank lines for better separation of sections in multiple test files. - Updated parameterization syntax in several test functions for clarity. - Ensured consistent formatting across all test files related to CPU instructions and memory.

Copilot

Pull Request Overview

Copilot reviewed 46 out of 47 changed files in this pull request and generated 6 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-13T01:29:15Z

tests/test_cpu.py

+    cpu.reg_a = value
+    cpu.reg_y = reg_y
+    memory[Processor.PC_INIT] = INS_STA_INY
+    memory[Processor.PC_INIT + 1] = 0x86


Hard-coded value 0x86 should use memory_zp parameter to be consistent with the test parameters and function signature.

Suggested change

memory[Processor.PC_INIT + 1] = 0x86

memory[Processor.PC_INIT + 1] = memory_zp

Copilot · 2025-10-13T01:29:15Z

tests/test_cpu.py

+        cpu.stack_pointer,
+        cpu.cycles,
+        memory[memory_location],
+    ) == (0xFCE4, Processor.SP_INIT, 4, 0xF0)


Hard-coded program counter value 0xFCE4 should use Processor.PC_INIT + size for consistency with other tests.

Suggested change

) == (0xFCE4, Processor.SP_INIT, 4, 0xF0)

) == (Processor.PC_INIT + 2, Processor.SP_INIT, 4, 0xF0)

Copilot · 2025-10-13T01:29:15Z

tests/test_cpu.py

+        cpu.stack_pointer,
+        cpu.cycles,
+        memory[memory_location],
+    ) == (0xFCE4, Processor.SP_INIT, 4, 0xF0)


Hard-coded program counter value 0xFCE4 should use Processor.PC_INIT + size for consistency with other tests.

Suggested change

) == (0xFCE4, Processor.SP_INIT, 4, 0xF0)

) == (Processor.PC_INIT + 2, Processor.SP_INIT, 4, 0xF0)

Copilot · 2025-10-13T01:29:16Z

src/m6502/processor.py

+        :return: None
+        """
+        self._write_byte(
+            self._read_byte(self._read_word(self._fetch_byte()) + self.reg_y),


This line appears to have incorrect logic - it reads a byte from a memory location plus reg_y, but should read the word from memory location and then add reg_y to get the final address.

Suggested change

self._read_byte(self._read_word(self._fetch_byte()) + self.reg_y),

self._read_word(self._fetch_byte()) + self.reg_y,

Copilot · 2025-10-13T01:29:16Z

src/m6502/memory.py

+        :rtype: int
+        :raises ValueError: Memory address is not valid
+        """
+        if 0x0000 < address > self.size:


Logic error in address validation - should be address < 0x0000 or address > self.size to properly check bounds.

Copilot · 2025-10-13T01:29:16Z

src/m6502/memory.py

+        :raises ValueError: Memory address is not valid
+        :raises ValueError: Size of value exceed byte size
+        """
+        if 0x0000 < address > self.size:


Logic error in address validation - should be address < 0x0000 or address > self.size to properly check bounds.

…ainer.json

… "Code example" to "Assembly example" and improve clarity. Remove redundant test file test_cpu_ins_lda.py.

hspaans added 3 commits September 6, 2025 01:10

Configure development environment with Python, add flake8 and tox set…

fc61f0d

…tings, and update documentation build tasks

Update flake8 plugins and add GitHub Copilot MCP configuration

9fb18d8

Copilot AI review requested due to automatic review settings October 13, 2025 00:45

hspaans linked an issue Oct 13, 2025 that may be closed by this pull request

Load/Store Operations #38

Open

6 tasks

hspaans self-assigned this Oct 13, 2025

Merge branch 'master' into 38-loadstore-operations

958dc13

Copilot AI reviewed Oct 13, 2025

View reviewed changes

hspaans added 3 commits October 13, 2025 00:50

Update pylock.toml for editable package installation, modify tox vers…

89967c0

…ion, and enhance linting configuration

Simplify the C matrix to ubuntu-latest and Python 3.11, 3.12, and 3.13

9ed5e70

Copilot AI review requested due to automatic review settings October 13, 2025 01:28

Copilot AI reviewed Oct 13, 2025

View reviewed changes

hspaans added this to the v0.1.0 - Initial m6502 implementation milestone Oct 13, 2025

hspaans marked this pull request as draft October 13, 2025 01:31

hspaans added 4 commits October 18, 2025 15:51

Voeg ms-python.black-formatter toe aan de VSCode extensies in devcont…

fcf2c76

…ainer.json

Correcting typo from LDT to LDY

94df872

Update yamllint findings in codeql.yml

9811aaf

Refactor documentation in processor.py and test_cpu.py to standardize…

a38608f

… "Code example" to "Assembly example" and improve clarity. Remove redundant test file test_cpu_ins_lda.py.

	set_env.COVERAGE_FILECOVERAGE_PROCESS_START = "{tox_root}{/}pyproject.toml"
	set_env.COVERAGE_FILE_COVERAGE_PROCESS_START = "{tox_root}{/}pyproject.toml"

		after the opcode. The memory location is a two-byte address that can

	after the opcode. The memory location is a two-byte address that can
	calculated by adding the X register to the two-byte address specified
	after the opcode. The base address can range from 0x0000 to 0xFFFF.

	eval("self._ins_" + self._OPCODES[opcode] + "_" + self._ADDRESSING[opcode] + "()") # noqa: PLW123
	method_name = "_ins_" + self._OPCODES[opcode] + "_" + self._ADDRESSING[opcode]
	getattr(self, method_name)()

	memory[Processor.PC_INIT + 1] = 0x86
	memory[Processor.PC_INIT + 1] = memory_zp

	) == (0xFCE4, Processor.SP_INIT, 4, 0xF0)
	) == (Processor.PC_INIT + 2, Processor.SP_INIT, 4, 0xF0)

	self._read_byte(self._read_word(self._fetch_byte()) + self.reg_y),
	self._read_word(self._fetch_byte()) + self.reg_y,

Refactor CPU instruction tests and enhance development setup #53

Are you sure you want to change the base?

Refactor CPU instruction tests and enhance development setup #53

Uh oh!

Conversation

hspaans commented Oct 13, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants