3. Add Core.Test c++ test project; add tests for CodeDataLogger#78
Draft
culix-7 wants to merge 23 commits intoSourMesen:masterfrom
Draft
3. Add Core.Test c++ test project; add tests for CodeDataLogger#78culix-7 wants to merge 23 commits intoSourMesen:masterfrom
Core.Test c++ test project; add tests for CodeDataLogger#78culix-7 wants to merge 23 commits intoSourMesen:masterfrom
Conversation
needed to build Utilities reference for Core.Test project. fixes: ``` unresolved external symbol __imp_timeBeginPeriod unresolved external symbol __imp_timeEndPeriod ```
so test project can find the output files it needs
add a simple nullcheck wrapper around the `debugger` instance, to make it easy to test this class without having to mock or implement the full Debugger class. add simple constructor checks
write docstring to clarify return values then fix them so they pass all tests
the full workflow of: create, mark bytes, save, load works.
technically if you have a `_memSize` bigger than the size of max int, then `i` would overflow and wrap, leading to bad things. That's probably unlikely, since you likely won't have a ROM file of size > max int. But this is more correct and costs nothing, so we might as well fix it.
for easy reuse
culix-7
changed the title
Core.Test c++ test project; add tests for CodeDataLoggerCore.Test c++ test project; add tests for CodeDataLogger
1. Doing a build + clean on UI, and then a publish might have been making the cloud runners build twice. 2. Change windows to publish `UI` project, not whole solution. Trying to publish the whole solution breaks the publish step: ``` D:\a\Mesen2\Mesen2\Core.Test\Core.Test.vcxproj(56,3): error MSB4278: The imported file "$(VCTargetsPath)\Microsoft.Cpp.Default.props" does not exist and appears to be part of a Visual Studio component. This file may require MSBuild.exe in order to be imported successfully, and so may fail to build in the dotnet CLI. `````` , because `dotnet publish` doesn't know how to handle c++ projects, while msbuild does. Setting to publish just the main `UI` project is the 'more clean and correct' setting; to publish the final output. We want to keep the Core.Test project included in the build, so we can run the tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hello, thank you very much for creating and sharing Mesen. It is excellent.
Would it be helpful if I offer to add some tests to the Mesen codebase? Would that make refactoring or bugfixing any easier, if we can have some confidence about parts of the codebase not breaking?
As a proof of concept I created some simple tests for the
CodeDataLoggerclass.Overall the
CodeDataLoggerworks well. The tests I wrote confirm that it has correct behaviour and always does the right thing.I created an integration test that creates a new
CodeDataLogger, marks some bytes and code and data, saves the file to disk, and then reads and loads the file. It is able to confim the data is what we expect.I was able to also create a few tests that cause
CodeDataLoggerto crash or die in unlikely scenarios - basically by passing innullptror invalid lengths and offsets that cause it to read outside of normal memory bounds.These are unlikely scenarios. But I added a few bounds checks and null checks just to make the class more robust.
The most difficult part of creating the tests was not anything about writing the tests themselves - it was getting the new project set up and configured correctly in Visual Studio. So I thought that sharing a new test project with any small number of tests added might create a useful starting point. Anyone can then add more tests more easily for any part of the code that they want.
Thank you for your time.
Before:
After: