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

Various failures in compiling current MAME integrated tests #7716

Open
angelosa opened this issue Jan 28, 2021 · 5 comments
Open

Various failures in compiling current MAME integrated tests #7716

angelosa opened this issue Jan 28, 2021 · 5 comments
Labels
needs design implementation details needs to be properly addressed question tools

Comments

@angelosa
Copy link
Member

angelosa commented Jan 28, 2021

Steps I tried to launch unit testing from root MAME source, host Win 8.1 (correct me if I'm wrong)

make TOOLS=1 <etcetera>
make tests

With default regtests.mak and python 3.8.6 only jedutil unittest is launched (?), and all linked unit test fails:

GCC 10.2.0 detected
Running jedutil unittest
python.exe regtests/jedutil/jedtest.py
Test 18cv8_bi-directional_io.jed failed
Test 18cv8_combinatorial_feedback.jed failed
[...]
regtests/regtests.mak:56: recipe for target 'jedutiltest' failed
make: *** [jedutiltest] Error 1

If I invert jedutiltest and chdmantest order in REGTESTS set at line 45 then latter gets executed, except the underlying output is full of ...

createcd_cue_empty - info output differs
expected: d3606248ea42b39ad0f7a6d32d9a8c234e6113d1 found: e5aa14b40029fbe4dfd87d
ea371d664b183e6c60
createcd_cue_empty - SHA1 mismatch (output file)

And eventually ends by giving an exit code of 0 (???) go figure. ETA: looks like that the unit test make option stops at first "macro test" encountered, inconvenient imho.

It also doesn't seem like that the catch C++ unit test is involved at all, notably because there are tests defined. I honestly ignore if or how anything really compiles that part, definitely doesn't seem executed by just make tests and seems abandoned by itself (4 years old).

Bottom line if we want to link these to a CI action then we need to fix or ditch the two/three tests suites involved, tbh other than the obvious python scripts that can benefit from overall improvement I'm not even sure about the user friendliness of current catch2 library vs. something like CppUnit, not an expert of C++ unit testing tbh.

@angelosa angelosa added question needs design implementation details needs to be properly addressed labels Jan 28, 2021
@cuavas
Copy link
Member

cuavas commented Jan 29, 2021

Catch2 is generally easier to code for than CppUnit, and it supports templated tests, groups, etc. It’s also lightweight with minimal dependencies. I think choosing Catch2 isn’t an issue on its own. The issue is that the unit tests are more-or-less abandoned and never run.

@angelosa angelosa changed the title Various failures in compiling current MAME unit tests Various failures in compiling current MAME integrated tests Feb 2, 2021
@angelosa angelosa added the tools label Feb 14, 2021
@firewave
Copy link
Contributor

These are blackbox tests for the binaries so Catch2 and CppUnit make no sense at all.

pytest also does not seem to make sense since it uses a file structure as input/output I am not sure how to figure those in there and it seems unnecessary. The output could be greatly improved though. A tiny bit of documentation on the structure would not hurt either.

And the reason that it always exits with 0 is just because these were always run by hand and never by a tool/workflow so it simply didn't matter.

All the tests still pass with an official chdman from 0.160. Writing a small script to pull all versions to figure out when the various divergences started should be easy.

@angelosa
Copy link
Member Author

angelosa commented Jan 4, 2025

pytest also does not seem to make sense since it uses a file structure as input/output I am not sure how to figure those in there and it seems unnecessary.

As long as any kind of integrated tests is executed and gives some kind of confidence about non-trivial functions is okay at this point. You can do your own thing or use any library in C++, Python, Rust, NodeJS or whatever else I don't really care in any way or form.
I care more about:

  1. It can access an assert form in an easy to use single liner interface;
  2. That it can be executed regardless of having a build ready or not;
  3. It can filter file/directory patterns from the CLI interface for standalone execution (so not necessarily tools alone);
  4. That is eventually executable by a CI action;

All the tests still pass with an official chdman from 0.160. Writing a small script to pull all versions to figure out when the various divergences started should be easy.

Given the late ping about a 4+4 year old issue and #7411 do you plan doing it in the short term? I don't wanna sound harsh, but this is becoming attic league, possibly throw heaps of Python 3.13/3.14 linting errors at this point ...

@firewave
Copy link
Contributor

firewave commented Jan 7, 2025

Given the late ping about a 4+4 year old issue and #7411 do you plan doing it in the short term? I don't wanna sound harsh, but this is becoming attic league, possibly throw heaps of Python 3.13/3.14 linting errors at this point ...

Hopefully. I am not gonna address your requirements though because that would be a total rework from the ground up which needs to be totally generic which is completely out of the scope what I was trying to do here (making sure CHD is a stable/deterministic archiving solution).

I will do what I outlined above so we end up with an improved implementation, a detailed list of the introduced regressions and something that could be integrated into the CI. If I get around to it I will take a look at the improvements made since to create a list of tests necessary to improve the coverage.

@firewave
Copy link
Contributor

Good news, everybody.

I collected all the versions of chdman since the last test was run and it turns there is nothing which affects the actual CHD SHA1. There is a long ago change related to metadata and some two more recent changes where the size of the whole CHD changed. It looks like the compression changed slightly - or it is related to #13048.

I will prepare PRs for the update of the expected output so the test can at least be run manually and pass.

Improvements to the test itself will come afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs design implementation details needs to be properly addressed question tools
Projects
None yet
Development

No branches or pull requests

3 participants