search

mt-mods / technic

Technic mod for Minetest
18 stars 25 forks source link

Extended Mineunit tests #208

Closed S-S-X closed 3 years ago

S-S-X commented 3 years ago

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 https://github.com/mt-mods/technic/pull/208#issuecomment-946919098 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 🤣):

$ mineunit | grep 'as reference, this can lead to unxpected results' | wc -l
836
github-actions[bot] commented 3 years ago
Mineunit failed regression tests, click for details ### Regression test log for technic: ``` ●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●◼●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●◌●●●●● 68 successes / 1 failure / 0 errors / 1 pending : 15.45806 seconds Pending → spec/supply_converter_spec.lua @ 99 Supply converter building overloads network spec/supply_converter_spec.lua:99: overload does not work with supply converter Failure → spec/hv_network_spec.lua @ 104 HV machine network charges battery box spec/hv_network_spec.lua:107: Expected (number) 0 to be more than (number) 1000 ```
S-S-X commented 3 years ago

Finding out what causes single test case to fail with pull_request action but succeed with push action. Tests are exactly same and all file contents are exactly same, pull_request took ~15.5 seconds and push took ~13.3 seconds.

So testing locally with cpulimit -l 2 which took 17 seconds to finish, 4 tests failed. All tests have something to do with results depending on time passed. For example not enough ores smelted or grinded, network was executed only few times instead of what was expected. Running again few times results vary based on how long it took to execute tests.

This most probably has something to do with max_lag.lua and max lag throttling, lag values were faked before but now mod is loaded completely and lag values use actual real world time instead of faking it. Not only that, same reason also causes per network lag to be significantly higher and cause network execution to skip cycles. Have to fake both for consistent test results that wont be affected by real world limitations.

Answer: Implemented in https://github.com/mt-mods/mineunit/pull/27 Tested with cpulimit and limited a lot, over 2 minutes executing tests and all succeed:

69 successes / 0 failures / 0 errors / 1 pending : 128.714934 seconds
S-S-X commented 3 years ago
Long list of things tests are currently doing: * CNC API * Machine control * returns product item string * configures new program * fails configuring invalid program * machine is enabled by default * disables machine * enables machine * manufactures products * fails manufacturing products * Machine registration * registers example machine * CNC formspec interaction * allows placing materials * allows placing materials protected * disallows placing materials protected * does not check protection for noop actions * checks protection for programming * sets metadata on form submit * manufactures products * updates old machines * CNC digiline API * handles invalid messages * sets program and size * disables machine * enables machine * returns programs * Technic CNC * technic:cnc * produces items * uses energy * technic:cnc_mk2 * produces items * uses energy * Power network building * cable building * does not crash * is added to network * adds all network nodes * adds connected machines to network without duplicates * cable building to machine * does not crash * is added to network * adds all network nodes * adds connected machines to network without duplicates * does not add unconnected cables to network * does not duplicate already added machine * machine building * does not crash * is added to network without duplicates * does not remove connected machines from network * does not remove network * does not add cables to network * overloads network * cable building between networks * does not crash * removes network * cable cutting * does not crash * is removed from network * removes connected cables from network * removes connected machines from network * removes network * cable digging below machine * does not crash * is removed from network * removes connected machines from network * does not remove network * keeps connected machines in network * machine digging * does not crash * is removed from network * does not remove other nodes from network * does not remove connected machines from network * does not remove network * HV machine network * executes network * kills network when switching station disappear * charges battery box * smelts ores * grinds ores * LV machine network * executes network * kills network when switching station disappear * charges battery box * smelts ores * grinds ores * comperess sand * cuts power when generators disappear * Power network helper * network lookup functions * does not fail if network missing * returns correct position for network * returns correct network for position * returns nil tier for empty position * returns correct tier for switching station position * network constructors/destructors * creates network * builds network * does not add duplicates to network * removes network * Power network timeout functions technic.touch_node and technic.get_timeout * returns zero if no data available * returns timeout if data is available * technic.merge_networks * function behavior * merges networks * merges networks again * network building behavior * stops network building after first iteration * continues network building * merges with second network * finishes network build * Supply converter * building * does not crash * is added to network without duplicates * does not remove connected machines from network * does not remove networks * does not add cables to network * overloads network * digging * does not crash * is removed from network * does not remove other nodes from network * does not remove network * formspec * is protected
S-S-X commented 3 years ago

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 commented 3 years ago

@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:

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

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. Especially I'd like to integrate this into #230, #232 and #233 as all those introduce API changes for upcoming release, from there it would be good to get immediate notification if changing anything might also affect API.

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. For example end, local varname, else and so on is excluded from counts mentioned. Currently master branch has about 9219 lines of code counted like that (includes some of technic_worldgen and technic_cnc).

BuckarooBanzay commented 3 years ago

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

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...?

S-S-X commented 3 years ago

IMO: More tests are better, updating the tests if there are big changes isn't really much more work (hopefully)

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.

github-actions[bot] commented 3 years ago
Click for detailed source code test coverage report ### Test coverage report for Technic CNC 79.01% in 10/14 files: ``` File Hits Missed Coverage ----------------------------------------------------- programs.lua 263 0 100.00% materials/basic_materials.lua 17 0 100.00% materials/default.lua 177 4 97.79% cnc.lua 50 3 94.34% materials/init.lua 13 1 92.86% formspec.lua 103 8 92.79% digilines.lua 39 8 82.98% init.lua 19 6 76.00% api.lua 160 83 65.84% pipeworks.lua 25 13 65.79% materials/technic_worldgen.lua 0 25 0.00% materials/moreblocks.lua 0 29 0.00% materials/ethereal.lua 0 37 0.00% materials/bakedclay.lua 0 13 0.00% ``` ### Test coverage report for technic chests 45.24% in 6/6 files: ``` File Hits Missed Coverage ---------------------------------- chests.lua 98 18 84.48% init.lua 34 18 65.38% register.lua 84 78 51.85% formspec.lua 76 93 44.97% inventory.lua 10 100 9.09% digilines.lua 2 61 3.17% ``` ### Test coverage report for technic 64.48% in 117/117 files: ``` File Hits Missed Coverage -------------------------------------------------------------- max_lag.lua 12 0 100.00% machines/register/init.lua 22 0 100.00% machines/register/grinder.lua 6 0 100.00% machines/register/freezer_recipes.lua 13 0 100.00% machines/register/freezer.lua 6 0 100.00% machines/register/extractor.lua 6 0 100.00% machines/register/electric_furnace.lua 6 0 100.00% machines/register/compressor.lua 6 0 100.00% machines/register/centrifuge.lua 6 0 100.00% machines/other/init.lua 8 0 100.00% machines/MV/solar_array.lua 9 0 100.00% machines/MV/init.lua 17 0 100.00% machines/MV/grinder.lua 8 0 100.00% machines/MV/generator.lua 9 0 100.00% machines/MV/freezer.lua 8 0 100.00% machines/MV/extractor.lua 8 0 100.00% machines/MV/electric_furnace.lua 8 0 100.00% machines/MV/compressor.lua 8 0 100.00% machines/MV/centrifuge.lua 14 0 100.00% machines/MV/battery_box.lua 17 0 100.00% machines/MV/alloy_furnace.lua 8 0 100.00% machines/LV/solar_array.lua 8 0 100.00% machines/LV/init.lua 17 0 100.00% machines/LV/grinder.lua 9 0 100.00% machines/LV/generator.lua 9 0 100.00% machines/LV/electric_furnace.lua 8 0 100.00% machines/LV/compressor.lua 13 0 100.00% machines/LV/battery_box.lua 15 0 100.00% machines/LV/alloy_furnace.lua 8 0 100.00% machines/HV/solar_array.lua 8 0 100.00% machines/HV/init.lua 12 0 100.00% machines/HV/grinder.lua 8 0 100.00% machines/HV/generator.lua 9 0 100.00% machines/HV/electric_furnace.lua 8 0 100.00% machines/HV/compressor.lua 8 0 100.00% machines/HV/battery_box.lua 17 0 100.00% legacy.lua 33 0 100.00% items.lua 128 0 100.00% crafts.lua 133 0 100.00% ../technic_worldgen/nodes.lua 109 0 100.00% ../technic_worldgen/crafts.lua 103 0 100.00% ../technic_worldgen/config.lua 9 0 100.00% ../technic_cnc/programs.lua 263 0 100.00% ../technic_cnc/materials/technic_worldgen.lua 32 0 100.00% ../technic_cnc/materials/init.lua 14 0 100.00% ../technic_cnc/materials/basic_materials.lua 17 0 100.00% machines/LV/led.lua 73 1 98.65% ../technic_cnc/materials/default.lua 178 4 97.80% machines/register/compressor_recipes.lua 36 1 97.30% machines/LV/geothermal.lua 75 3 96.15% config.lua 49 2 96.08% machines/LV/solar_panel.lua 42 2 95.45% machines/register/solar_array.lua 40 2 95.24% machines/register/cables.lua 208 12 94.55% ../technic_cnc/cnc.lua 50 3 94.34% tools/init.lua 13 1 92.86% machines/LV/water_mill.lua 67 6 91.78% machines/network.lua 323 31 91.24% machines/LV/lamp.lua 111 13 89.52% machines/register/grindings.lua 42 5 89.36% ../technic_worldgen/overrides.lua 39 5 88.64% init.lua 29 4 87.88% machines/register/grinder_recipes.lua 106 16 86.89% ../technic_worldgen/rubber.lua 64 10 86.49% ../technic_worldgen/init.lua 19 3 86.36% ../technic_worldgen/oregen.lua 155 28 84.70% machines/register/machine_base.lua 164 41 80.00% ../technic_cnc/formspec.lua 88 23 79.28% ../technic_cnc/init.lua 19 6 76.00% machines/register/recipes.lua 62 20 75.61% tools/flashlight.lua 65 21 75.58% machines/switching_station.lua 76 25 75.25% machines/register/centrifuge_recipes.lua 21 7 75.00% machines/register/battery_box.lua 219 73 75.00% ../technic_cnc/api.lua 194 66 74.62% effects.lua 5 2 71.43% radiation.lua 245 100 71.01% machines/LV/extractor.lua 11 5 68.75% machines/MV/wind_mill.lua 46 22 67.65% machines/other/coal_furnace.lua 2 1 66.67% machines/supply_converter.lua 93 48 65.96% ../technic_cnc/pipeworks.lua 25 13 65.79% machines/switching_station_globalstep.lua 40 21 65.57% machines/register/alloy_recipes.lua 28 15 65.12% machines/other/injector.lua 72 39 64.86% machines/MV/hydro_turbine.lua 43 26 62.32% machines/MV/tool_workshop.lua 56 34 62.22% machines/register/generator.lua 122 88 58.10% register.lua 21 19 52.50% tools/cans.lua 53 48 52.48% machines/power_monitor.lua 40 38 51.28% machines/other/coal_alloy_furnace.lua 63 63 50.00% machines/LV/music_player.lua 46 46 50.00% tools/mining_lasers.lua 39 40 49.37% machines/other/constructor.lua 67 69 49.26% machines/MV/cables.lua 10 11 47.62% machines/LV/cables.lua 10 11 47.62% machines/init.lua 49 54 47.57% tools/vacuum.lua 19 21 47.50% tools/tree_tap.lua 24 27 47.06% machines/HV/cables.lua 9 11 45.00% machines/HV/forcefield.lua 101 157 39.15% machines/HV/quarry.lua 124 217 36.36% machines/HV/nuclear_reactor.lua 116 208 35.80% machines/register/common.lua 39 75 34.21% machines/register/alloy_furnace.lua 10 20 33.33% tools/chainsaw.lua 40 83 32.52% tools/multimeter.lua 71 149 32.27% tools/sonic_screwdriver.lua 18 40 31.03% machines/other/frames.lua 184 445 29.25% helpers.lua 33 83 28.45% tools/mining_drill.lua 76 226 25.17% machines/other/anchor.lua 14 74 15.91% machines/compat/digtron.lua 2 11 15.38% tools/prospector.lua 16 92 14.81% util/throttle.lua 1 10 9.09% machines/register/extractor_recipes.lua 6 65 8.45% ``` ### Raw test runner output for geeks: CNC: ``` ●●●●●●●●●●●●●●●●●●●●●● 22 successes / 0 failures / 0 errors / 0 pending : 0.146917 seconds ``` Chests: ``` W: Configuration: invalid key exclude_textures W: Configuration: invalid key validate_textures ●●●●● 5 successes / 0 failures / 0 errors / 0 pending : 0.028 seconds ``` Technic: ``` ●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●◌●●●●● 169 successes / 0 failures / 0 errors / 1 pending : 6.642712 seconds Pending → spec/supply_converter_spec.lua @ 78 Supply converter building overloads network spec/supply_converter_spec.lua:78: overload does not work with supply converter ```
S-S-X commented 3 years ago

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?