-
Notifications
You must be signed in to change notification settings - Fork 25
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
Remove leftovers from frames, add README.md for Mineunit spec dirs #355
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mineunit tests are supposed to load in as much as possible instead of simply reflecting default config.
Integration tests AFAIK use default configuration but is simply just validating that mod can be loaded without crashing.
Flashlight tool for example is actually tested with Mineunit and not having it causes this:
Failure → spec/tools_spec.lua @ 275
Technic power tool Flashlight is registered
spec/tools_spec.lua:276: Expected nil to be hash table
Error → spec/tools_spec.lua @ 283
Technic power tool Flashlight charge is used
spec/tools_spec.lua:286: attempt to index upvalue 'itemdef' (a nil value)
enable_frames could be removed if there's no references to it anymore, should not be and just managed to get through when those were removed:
Wouldn't start limiting tests to only default stuff, if there's special need to actually run tests for default configuration it can be done as a separate configuration test utilizing multiple configuration files one for each scenario (like default, all enabled, all disabled, etc.). For information, currently things that are actually tested in one way or another:
|
bf7a18b isn't supposed to be part of this PR |
Hello, I understand neither messages sent by S-S-X |
please disregard this PR, how do I delete it? |
Can be just closed or if you like you could also just revert technic/spec/fixtures/technic.conf to version in master branch and then we merge that simple README.md change, it isn't big change but it is still good as it would remove unneeded legacy stuff. I mean just |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good for me: simple README.md cleanup + adds note to communicate spec directory purpose.
Frames were removed here: #290
Same note could be also added to CNC and chests spec directories but fine for me either way.
Noticed that enable_frames was also on technic.conf for Mineunit so removed it from there too. Searched a bit if there's anything else about frames and didn't found anything, should be clean now. |
as far as I'm able to tell enable_frames doesn't do anything, I removed that, and also updated technic/spec/fixtures/technic.conf to reflect the defaults as per technic/config.lua
low impact PR