Use of screeps-server-mockup & other unit testing tools in "starter" package

[kotarou]

As per PR #113 and PR #114, but I am splitting this into a new issue for discussion.

Testing frameworks and tools are useful for mature codebases, but cause installation issues and present a hurdle to first-time users, who already have to deal with the hassle of getting typescript running.

I propose that all testing tools are removed from the starter package, and intead added into a wiki page (or the docs) connected to this repo detailing how to build, run, and utilize tests.

Speficially, the following tools would be removed:

• chai
• mocha
• screeps-server-mockup
• sinon
• related @types packages

[eduter]

AFAIK, all test-related problems are caused by screeps-server-mockup's dependencies. I see no need to remove unit testing. Having no unit test framework configured would increase the barrier, possibly making even fewer people test their code. To be fair, very few do already, but the rest can just ignore those dependencies and configs without issues and, if one day they want to start testing, there's no setup needed.

[tcdejong]

Having some overhead from unused packages like sinon/mocha/chai is totally acceptable for a starter kit, in my mind. The installation hurdles from the server mockup not so much.

I'd side with eduter and include the packages that do not cause installation difficulties so that the testing framework is there when people are ready, but removing the server mockup to streamline installation.

#118 is again struggling with node-gyp, which is required for the server mockup.

[pyrodogg]

So, I took a dive into the screeps-server-mockup package over the past week since removing it from the starter project due to #118. It's now a Screepers project and works with the latest Screeps version!

I reached out to the author, Hiryus, regarding the projects deprecated status and the issues people were having with the TS starter package. They no longer have time to maintain it and weren't aware that it was even referenced by this starter package. I put in a bit of time to get it working and they agreed to transfer it to the Screepers org.

From what I found, I don't think the issue was with node-gyp being required per se, it's that the mockup project was referencing older versions of the Screeps private server which may no longer be buildable without special handling or backdating other packages.

I believe if the server-mockup package is kept up to date with major version changes in the Screeps private server this shouldn't be a significant overhead.

[brooksvb]

I'm not sure I understand the status of this issue. I noticed when using this project starter that the screeps-server-mockup dependency was missing. It was removed to prevent some errors, but now its ownership has been transferred and it is up-to-date, right?

If we are concerned about people using the starter but having an outdated version of the dependency, a post-install script could be included to pop up a warning message of "Please double-check this package version; it could behave differently from the game environment if out of date"

[pyrodogg]

Since I've gotten the server mockup project updated, I'm also working on actually using it more in my own Screeps project. I'm liking it more than having to deal with mocks since it actually runs a server instance nothing needs to be mocked.

I think it should be re-introduced to the starter template, as it was only removed due to the fact it was causing errors, and is now fixed.

[tcdejong]

Thanks @pyrodogg for contacting the previous maintainer and working on the mockup.

That said, I'm still not in favor given the Python requirement. Running two versions of Python is annoying because you often have conflicts regarding which one is called python in PATH on Windows. If node-gyp is now compatible with both 2 and 3 that makes things easier, but requiring Python is just another source for errors. I think that is undesirable for a starter kit.

Especially because many users are relatively inexperienced programmers, I'm not sure how many of them would even go through unit testing to begin with. I never see much about it in the Slack channels except for a few "veterans". I'm convinced that users who are ready for unit testing are also proficient enough to add the package themselves.

screeps-server-mockup may have been removed because of installation errors, but I question it's value for the starter kit to begin with. Personally I think it's not worth the trade off.

[eduter]

Maybe the starter kit could have instructions of how to add it, instead of including it. Or maybe create a fork, including the server mockup, plus some realistic examples of how to use it in a useful way. I'm curious about how you're using it, @pyrodogg. You say nothing needs to be mocked, but how do you test, let's say, a repairer? Don't you need to somehow mock at least a creep and a structure?

[pyrodogg]

Yea, after a couple more days, I think it's reasonable to remove the heavier integration testing from the base install of the starter kit. Not everyone is going to be interested in running a local private server.

Also, my "mock" v. "no mock" comparison wasn't fair. The integration testing isn't "no setup", unless you want to automate testing the start-up of your bot in a static scenario. Otherwise it's going to require some knowledge of setting up rooms with different scenarios.

You need to create model scenarios in-game that you want to test by adding game objects with specific configuration, then ticking the server. That is, add a hostile creep to your room with a tower and see observe what happens. Unless that's all you really add it's not really an isolated test of the tower targeting. So mocked unit testing would be preferred there.

I'm still getting familiar with both the unit testing of screeps and this integration testing project.

I think #120 can be merged, and I'll look at further cleaning up this project to remove the remnants of integration testing, leaving a plug for how to install it if users want it. I'm also adding to the server-mockup documentation.

[jallman112]

test/integration/helper.ts is still referencing screeps-server-mockup, which surprised me the first time I ran npm run test. Looks like it's also still discussed in docs/in-depth/testing.md.

FWIW, as an experienced developer who's just getting into the larger JS/TS ecosystem, I value the opinionated choices made in this starter package. I'd rather learn one way to test, and then go looking around to see if I like something better, instead of starting with nothing and having to choose myself what testing framework to integrate with this project and how to do it from scratch.

[brooksvb]

I also agree that having some basic even if opinionated choices are very nice as a starting point. If I decide I want to use a different testing framework, I can and will do that. It's also greatly helpful to beginners who have some of the difficult groundwork laid out to at least get to coding quicker.

[djD-REK]

Hey @brooksvb and @jallman112 😄

test/integration/helper.ts is still referencing screeps-server-mockup, which surprised me the first time I ran npm run test.

Looks like it's also still discussed in docs/in-depth/testing.md.

It looks like pull request #124 would close this issue #117 by adding docs about integration testing to that testing.md file, updating the README, and changing the package.json as follows:

   "test": "npm run test-unit && npm run test-integration",

to

    "test": "npm run test-unit",

FWIW, as an experienced developer who's just getting into the larger JS/TS ecosystem, I value the opinionated choices made in this starter package. I'd rather learn one way to test, and then go looking around to see if I like something better, instead of starting with nothing and having to choose myself what testing framework to integrate with this project and how to do it from scratch.

I agree 👍 I'd a million times rather just be able to npm run test and then start adding tests to an existing template, with the option of changing frameworks later if needed.

My opinion on merging #124 is to make screeps-server-mockup a dependency

Personally, I think making https://github.com/screepers/screeps-server-mockup a required dependency again is fine for that reason. In that case, #124 would need to be tweaked to add screeps-server-mockup to the package.json file ( reverting #109 ) and to change the instructions.

I think it would be helpful to still include a modified version of those instructions in #124

But presently (5/1/2020) yarn test doesn't work out of the box, so #124 should be merged sooner rather than later, either as-is (removing integration testing by default) or a modified version (re-adding the screeps-server-mockup dependency).

Because currently I get that error from helper.ts when I run npm test:

test/integration/**/*.test.ts → dist/test-integration.bundle.js...
created dist/test-integration.bundle.js in 2.2s
Error: Cannot find module 'screeps-server-mockup'

Cheers! 😄

[pyrodogg]

#124 has now been merged into master.

Out of the box, npm test will only run the unit test scripts.

Instructions have been added to the testing docs regarding how to install screeps-server-mockup and enable integration testing.

If npm run test-integration is run without following instructions in documentation, a message is printed directing user to documentation.

I think integration testing can be beneficial. I also think these simple steps should be enough for interested users to get started with it without placing the burden on everyone to have tools for building the server components.

[djD-REK]

Solid, thanks!

…

On Sat, May 2, 2020, 6:11 AM Skyler Kehren ***@***.***> wrote: #124 <#124> has now been merged into master. Out of the box, npm test will only run the unit test scripts. Instructions have been added to the testing docs <https://github.com/screepers/screeps-typescript-starter/blob/master/docs/in-depth/testing.md#integration-testing> regarding how to install screeps-server-mockup and enable integration testing. If npm run test-integration is run without following instructions in documentation, a message is printed directing user to documentation. I think integration testing can be beneficial. I also think these simple steps should be enough for interested users to get started with it without placing the burden on everyone to have tools for building the server components. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#117 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMV42QV6W4LH6HZ2JZW7T3TRPPWTTANCNFSM4JOLMD2A> .