search

mt-mods / wrench

Other
2 stars 4 forks source link

Mineunit tests #4

Open S-S-X opened 2 years ago

S-S-X commented 2 years ago

Basic tests also showing bit more proper fixture use mentioned at https://github.com/mt-mods/wrench/pull/2#issuecomment-986022043

API is used in fixture, that's not exactly correct use and API call should be in spec instead but I guess you'll get the idea.

There's no coverage reporting or any comments added by bot if tests pass, failure comment is added with test logs if tests fail.

~~There's a lot of commits because this branch is currently based on wrench_support_extend All stuff is really in this single commit: https://github.com/mt-mods/wrench/commit/33348039cb3dda0de6fe8aae438ecff126c810b0~~ Changed base branch to be wrench_support_extend, if merged this can however be merged directly into master after #2 is done. I do recommend that because #2 is huge mixed changes already, bit over sized PR already.

github-actions[bot] commented 2 years ago

Mineunit regression tests failed, test log follows:

I:  Mineunit configuration loaded from  spec/mineunit.conf
I:  Mod configuration loaded from   mod.conf
I:  Setting modpath wrench  .
I:  Mineunit initialized, current modname is    wrench
I:  File not found: spec/fixtures/fonts
I:  File not found: spec/fixtures/fonts
I:  Settings object created from:   spec/fixtures/minetest.conf
I:  Running custom core.get_us_time with step increment: 100
I:  Loading fixture spec/fixtures/default.lua
I:  Loading source  init.lua
I:  Loading fixture spec/fixtures/bitchange.lua
I:  Executing register_on_mods_loaded functions

0 successes / 0 failures / 3 errors / 0 pending : 0.026271 seconds

Error → spec/init_spec.lua @ 34
Wrench bitchange:shop setup
spec/init_spec.lua:36: attempt to call method 'do_reset' (a nil value)

Error → spec/init_spec.lua @ 34
Wrench default:cobble setup
spec/init_spec.lua:36: attempt to call method 'do_reset' (a nil value)

Error → spec/init_spec.lua @ 34
Wrench no-op tool use setup
spec/init_spec.lua:36: attempt to call method 'do_reset' (a nil value)
S-S-X commented 2 years ago

~~Well yeah, also this depends on unreleased Mineunit updates: https://github.com/S-S-X/mineunit/pull/67 Will prob merge that soon too.~~ ✔️ done

OgelGames commented 2 years ago

Is this really needed? I think manual testing is enough for a single tool.

S-S-X commented 2 years ago

Is this really needed? I think manual testing is enough for a single tool.

No, automated testing of course is not needed. It is just to catch errors and skip a lot of manual testing.

OgelGames commented 2 years ago

I had a closer look at this, and it only tests the core functionality of the wrench - something that isn't likely to change at all, especially not frequently enough to need automated tests.

S-S-X commented 2 years ago

I had a closer look at this, and it only tests the core functionality of the wrench - something that isn't likely to change at all, especially not frequently enough to need automated tests.

That sound very ignorant especially because things that are not supposed to change are exactly ones you do create automated tests.

S-S-X commented 2 years ago

I've reopened this because there was one approval already. It tells that one people does not like it while apparently at least 2, possibly 3 agrees (judging by reactions and approvals).

SwissalpS commented 2 years ago

I also think unit tests are good to confirm that core functionality has not changed with possibly irrelevant looking additions. The additions should also get tests because next editor will not remember to test everything manually each time.

Sometimes unit tests seem irrelevant and redundant as one has just written the code and the test with same mindset in mind. However, they can catch errors just as luacheck catches syntax errors or unused vars etc.