Skip to content

Port test suite back to unittest, and fix the easy bitrot - #353

Draft
PhoenixBound wants to merge 1 commit into
pk-hack:masterfrom
PhoenixBound:test-suite
Draft

PhoenixBound wants to merge 1 commit into
pk-hack:masterfrom
PhoenixBound:test-suite

Conversation

@PhoenixBound

@PhoenixBound PhoenixBound commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

CoilSnake's unit tests were ported from the Python standard library module unittest to nose pretty early on, in b5e61b0. Nose has received no commits in 10 years and has this written on its home page:

Nose has been in maintenance mode for the past several years and will likely cease without a new person/team to take over maintainership. New projects should consider using Nose2, py.test, or just plain unittest/unittest2.

So I didn't try installing nose or getting the existing test suite to work as-is, and decided to port it to one of the listed options.

Nose2 uses slightly different function names than the original Nose (snake_case names become camelCase names), and its documentation and maintainers generally recommend switching to other, more popular frameworks. Unittest is a bit verbose, but it's still a fairly mechanical translation from Nose, especially compared to what I've seen of pytest. pytest also has some support for incrementally switching from a unittest-based project if desired. And unittest has the benefit that you don't need to install a third-party dependency. So I went with unittest.

Other changes:

  • All checks for ROMs are now gated behind checks for the files existing, and the test is skipped otherwise.
  • Some "abstract test cases," as in test cases that rely on inheritance to fill in a class member to run the test with, will skip the test when the field isn't found, instead of failing. There might have been a better way to solve this but 🤷
  • Print statements in tests were removed
  • A few bitrotted tests were replaced with better versions (the compression test now sets an upper bound on the compressed size of the data, instead of checking for a long-outdated exact size).

There are a few tests that are still failing or misbehaving in nontrivial ways:

  • test_getitem_slice and test_setitem_slice in tests/model/common/test_blocks.py: appears to be a regression from Music support #270, which added support for indexing with negative numbers in slices. It's not just that negative numbers were expected to not work; there's unintuitive behavior going on like 0:-1 becoming 0:len(x)-1 and grabbing a non-empty sequence.
  • Various palette-related tests that expect 8 bits of precision to be preserved in color channels are failing. This behavior appears to have been changed 3 times; once in 4bf8737 specifically for EbColor.from_list, as part of town map bug fixes in CoilSnake 2.3; once in 3c1f732 specifically for EbColor.from_yml_rep, as a hack after many image-related problem reports with CoilSnake 2.3; and once in Fix palette bug when image file has less than the expected number of colours #260 for all the rest of the functions, which made all the EbColor factory functions consistent by reducing everything to 5 bits on construction. It's probably safe to update the tests here, but I haven't convinced myself of it yet
  • This isn't a failure necessarily, but EBP patching apparently must support "fixing" several "wrong" variants of the EarthBound ROM, and the tests for the fixing process require those "wrong" ROMs to be present in a specific folder. All we have of the ROMs are MD5 hashes (in the EBPatcher/CoilSnake source code) and CRC32 checksums (in the BPS patches from very old versions of EBPatcher). Running this test properly will probably require brute forcing all of the variant ROMs, which is probably easier for some of them than others... the wrong4 and wrong6 patches are huge.
  • The test runner also diagnosed a file handle leak in the project loading code, probably related to this attempt to support calling the same method with two very different inputs:

Tests can be run using python -m unittest discover -s tests. Marked as a draft for now because of the above issues, and because I haven't updated DEVELOPMENT.md with documentation, or removed references to Nose in setup.py.

The original nose is no longer maintained by its maintainers, only
nose2. And nose2 renames things to look more like unittest anyway, so
we might as well go with something standard.
I'm not *against* pytest, but... it was easy to get this to work without
a dependency, so why not keep doing things this way?

All checks for ROMs are now gated behind checks for the files existing.
A few tests are known to fail, due to changes in CoilSnake since the
tests were written (mainly block indexing and color conversion).

Tests can be run using `python -m unittest discover -s tests`.

This branch has not been deployed

No deployments
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.

1 participant