search

simon987 / Much-Assembly-Required

Assembly programming game
https://muchassemblyrequired.com
GNU General Public License v3.0
925 stars 87 forks source link

fix tests and configuration #201

Closed kevinramharak closed 5 years ago

kevinramharak commented 5 years ago

fixes #200 Ok this fixes tests while preserving most of the code base. I also had to adjust the sunfire fork configuration. I am not sure what that has to do with this problem except for a known issue noted here about the workingDirectory.

simon987 commented 5 years ago

Okay I just remembered looking at your changes that I made a FakeConfiguration class for this kind of stuff, it should be used everywhere in the test code. I will try to get a development environment running on my laptop and change it - even if it involves refactoring of existing code

Can you let me know what is your environment so I can test it afterwards?

kevinramharak commented 5 years ago

Lol, that makes sense in the files that currently use the FakeConfiguration class. I assumed you specifically wanted the actual configuration in the tests that cause problems because it only uses getters for config variables. The problem is that using a FakeConfiguration in these files is pretty much the same as using magic constants.

Is there any clean way to preserve the real configuration or at least part of it? It would be good practice to mock the configuration object, but then you cannot guarantee that the production configuration works right?

simon987 commented 5 years ago

Right, I haven't thought about that.. It shouldn't be able to compile if a change in the default config would break the tests.

simon987 commented 5 years ago

Thank you :)