Extended Mineunit tests #208
Merged
Conversation
This file contains 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
S-S-X
added
the
Testing needed
BuckarooBanzay
previously approved these changes
Sep 9, 2021
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Closed
Long list of things tests are currently doing:
|
S-S-X
force-pushed
the
extended-tests
branch
2 times, most recently
from
762484f to
9b405a9
Compare
|
Performance update for Mineunit was good too:
77 new tests are 77 nodes registered during tests (ignoring not_in_creative_inventory nodes). |
S-S-X
force-pushed
the
master
branch
2 times, most recently
from
ef88bf1 to
e5e4cbe
Compare
|
@OgelGames / @BuckarooBanzay to be able to add better tests for new API updates I would like to get this merged. So here's the question:
That's also reason why I'd like to have this while doing API upgrades for v2.0 release: guard new API against changes in interface and verify that updated API will actually follow specification. It seems like this should not cause any big troubles, been working fine over multiple changes and seems to be stable enough. I've tested this against all open pull requests and also for multiple stages in some branches. Executed LoC goes from 631 to 5890, this count include only lines of code that actually do something. |
S-S-X
added
Documentation
OgelGames
reviewed
Nov 1, 2021
IMO: More tests are better, updating the tests if there are big changes isn't really much more work (hopefully) Lets merge it and see where it leads to...? |
Should not be, at least I've been writing temporary tests for things because it is way faster than starting Minetest world, logging in and clicking things. Fixing tests takes time but compatibility/behavior/API has changed when it has to be done. If something goes wrong with Minetest and it crashes then you'll fix that thing and start over, if something goes wrong with Mineunit and it crashes then it happily proceeds to reset world and execute all of the remaining tests anyway. At least for me tests allows faster big changes to underlying API while providing confidence without a lot of manual testing, actually since I've started Mineunit most of my play testing has been single last verification instead of finding and correcting typos or other stupid mistakes. I'll review whole test set and clean up this updating for latest Mineunit where appropriate. |
Click for detailed source code test coverage reportTest coverage report for Technic CNC 79.01% in 10/14 files:Test coverage report for technic chests 45.24% in 6/6 files:Test coverage report for technic 64.48% in 117/117 files:Raw test runner output for geeks:CNC: Chests: Technic: |
|
Cleaned up reducing duplicate initialization code and verified that set passes with master, machine API updates (after renaming solar arrays) and tool API updates. @OgelGames / @BuckarooBanzay want to still review this stuff? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Generic and targeted extended tests for Technic modpack
Not sure if this is good or wanted, Mineunit supports a lot of things but still not even nearly everything.
This PR extends testing a lot but might be too fixed, it might be good thing as it enforces updating tests when there's big changes.
But for exact same reason it might not be wanted, dropping PR here anyway.
Current status
See comment #208 (comment) for complete list of current tests as of 957c2b8.
Old report: Test coverage report for technic 9.62% in 10/103 files
New report: Test coverage report for technic 64.48% in 117/117 files
Fixed annoyances
Also mineunit spamming a lot of warning
InvRef:get_list returning list src as reference, this can lead to unxpected results, that's mineunit issue and not technic or regression test issue.I do remember adding that warning to mineunit InvRef implementation just because I was not sure if actual minetest engine returns those as references or if it uses deep copies / copy-on-write objects of direct references to original object.
Test run on local machine, how many times these tests complain about possible "unxpected results" (yeah, just noticed typo too 🤣):