Skip to content
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

automated unit tests #997

Open
DreadKnight opened this issue May 31, 2016 · 20 comments
Open

automated unit tests #997

DreadKnight opened this issue May 31, 2016 · 20 comments
Labels
abilities The issue is related to an unit ability or more coding This issue requires some programming pipeline Affects how the project is being developed

Comments

@DreadKnight
Copy link
Member

DreadKnight commented May 31, 2016

The project will become more and more complex, especially due to all the unit abilities and their upgrades, so we need to perform automated tests in order to figure if most things still work out when something is being changed.

The framework picked is Jest:
https://jestjs.io
https://github.com/facebook/jest

Other candidates that were considered:
https://github.com/chaijs/chai

https://github.com/intuit/karate
https://github.com/jasmine/jasmine
https://github.com/mochajs/mocha

Here are some that foss projects can apply for
https://www.browserstack.com/open-source
https://crossbrowsertesting.com/open-source

@DreadKnight DreadKnight added coding This issue requires some programming abilities The issue is related to an unit ability or more pipeline Affects how the project is being developed labels May 31, 2016
@DreadKnight
Copy link
Member Author

DreadKnight commented May 31, 2016

So besides having the hipsterish name and advertising a dependence inducing beverage, Mocha uses a hexagon in the logo. Ancient Beast has a lot of hexagons on the combat field. Coincidence? Don't think so.

@cxong
Copy link
Contributor

cxong commented Jun 2, 2016

Based on that requirement, I suggest using https://github.com/chaijs/chai as the assertion library, since it also has a hexagonal logo 👌

@DreadKnight
Copy link
Member Author

Good suggestion, it will do the job. Seems they're using gitter too xD

@cxong
Copy link
Contributor

cxong commented Jun 4, 2016

So I found myself down a very deep rabbit hole and could go no further. The big problem is that Phaser is not meant to be run outside the browser, unlike the unit tests, which are run inside node. Unfortunately the current codebase is tightly coupled around Phaser, namely via the G variable.

There's like one guy who tried to get Phaser working in node. It works for a specific version (2.2.2 I think) and requires some hacking because Phaser works closely with the DOM, like document.createElement('canvas'). To get it to work, we need a fake DOM and canvas, and these are either flaky or take a lot of effort to set up (check out the epic dependencies for https://github.com/Automattic/node-canvas). The last thing we want for an open source project is to make it harder to contribute.

So that's the bad news; good news is that regular unit tests work fine, and I manage to remove prototype.js without too much drama. If you're still interested, then moving forward we can slowly decouple G from classes and start unit testing them. There's no reason why the classes in utility for instance need to use G. HexGrid for example could be refactored to take in a Game instance rather than use G, and use a mock Game inside the unit tests. Right now I have:

  • Removed prototype.js, convert to ES6 class syntax
  • Unit test framework using mocha + chai, runnable using mocha, npm test, or (TBA) grunt test and maybe an automatic testing via grunt watch

But we'll also need

  • A tool to compile javascript for the browser, to replace concat. Choices are browserify and webpack.
  • Babel to do ES6 polyfill

@DreadKnight
Copy link
Member Author

Sounds good, you can proceed.

I'm looking into browserify vs webpack atm.

Regarding Babel, I don't really care about Internet Explorer, so if it brings anything really useful to the table besides that without being too much of a pain, then perhaps.

@cxong
Copy link
Contributor

cxong commented Jun 4, 2016

Sure; if we avoid babel we can try to use minimal ES6, which means right now we only care about this: https://kangax.github.io/compat-table/es6/#test-class
Safari is a concern but apart from that ES6 browser support is pretty good.

@DreadKnight
Copy link
Member Author

DreadKnight commented Jun 4, 2016

Did research regarding browserify vs webpack. Seems that the later one is doing better and it also has hexagonal logo :D and gitter chat room. Always found browserify kinda confusing and doing certain things badly. There's also systemjs / JSPM, but meh. ES6 sounds good.

The main browser is Google Chrome, second class citizen is Firefox. So unless a browser is popular enough and a rather easy pick, won't go out of the way of supporting it.

For all the people insisting on using any sort of irrelevant or strange browsers, there will be downloadable version of the game eventually #158.

Regarding Ancient Beast website (even if rather underground), the visitor statistics are (even if probably very flawed) the following:

  • Google Chrome ~68%
  • Firefox ~18%
  • Safari ~7.5% (this sucka shows up twice, having an in-app version in the Google Analytics list)

Anyway, Chrome and Analytics being both Google products, the stats might as well be tampered with xD

@cxong
Copy link
Contributor

cxong commented Jun 15, 2016

I've spent another week attempting to get webpack to deploy the game, and there's still no end in sight, so I want to give up on this issue for now. The problem is that webpack works with the whole node / CommonJS / require() system, which cannot handle circular references, caused by the G variable which is used everywhere. I'm not sure if this is a hard requirement, but webpack seems to trip over bad javascript for me, like bad scoping and missing vars. That's a lot of stuff to untangle, and I don't think it's a good idea to do it all in one go anymore.

If we ever want to clean up the code so that it can be modular and testable, we'll need to:

  • Stop using G
  • One class per file
  • Start using a linter, and fix up all errors/warnings found
  • Try to isolate front-end-only libraries like jquery/phaser from game code as much as possible - they may need to be mocked for tests, not sure as I haven't got to this point yet

There's one more idea to try though; since the game has the gamelog functions, we could potentially have semi-automated gameplay tests via selenium - it will open a browser, run the game using gamelog, then check the status of creatures etc. It will need to be run manually but can be a good way to check PRs before merging, for example.

@cxong cxong removed their assignment Jun 15, 2016
@DreadKnight
Copy link
Member Author

I was wondering about your status on this. No worries, it's more important for now to keep things simpler with the pipeline in favor to focus on unit abilities and other stuff to get the release done asap.

That selenium sounds good, provided the game will have some sort of random AI to begin #542 , causing all sorts of moves to happen, hopefully revealing interesting scenarios and even bugs #997

@cxong
Copy link
Contributor

cxong commented Jun 16, 2016

Here's the work in progress, just FYI:
patch.zip

@DreadKnight
Copy link
Member Author

Would have been easier to check out if you made another branch just for this issue :)

@DreadKnight
Copy link
Member Author

Currently @ktiedt is doing work to remove Prototypejs #1216 and to convert to ES6 in the process.
You should have made a PR for that progress yourself long time ago it seems, @cxong as it was a step in the right direction (progress).

@DreadKnight DreadKnight added this to the 0.5 - Chimera milestone Oct 13, 2017
@DreadKnight
Copy link
Member Author

Nice related article regarding Riot's League of Legends https://engineering.riotgames.com/news/automated-testing-league-legends (thanks @GamesMaxed for sharing)

@DreadKnight
Copy link
Member Author

Testing suite, might come in handy https://www.cypress.io

@andretchen0
Copy link
Contributor

Any progress on this?

Unit tests would be handy for new code.

To move the discussion forward, I suggest Jest, simply because it appears to be the most popular testing framework on NPM and has pretty good docs. Maybe we work tests into **/__jest__/ folders to allow us to use a newer, handier framework, if one comes along.

That said, mocha/chai is fine for unit tests as well.

@DreadKnight
Copy link
Member Author

DreadKnight commented Apr 4, 2023

Any progress on this?

Unit tests would be handy for new code.

To move the discussion forward, I suggest Jest, simply because it appears to be the most popular testing framework on NPM and has pretty good docs. Maybe we work tests into **/__jest__/ folders to allow us to use a newer, handier framework, if one comes along.

That said, mocha/chai is fine for unit tests as well.

No progress on this stuff, sadly, as tests would be handy. Nice suggestions from you, I've peeked at Jest, looks good to me.
I know someone working as a company doing unit testing, said I should poke her about it a while ago, maybe it wouldn't hurt to peek her brain at least about it or even see if she's down for contributing on this stuff, although a people just say they want to help and never actually do it... will see how that goes. Regardless, I should aim for more coding contributors.

@andretchen0
Copy link
Contributor

You mentioned you were going to talk to a friend. I don't want to jump the gun; would you accept a PR with Jest or another framework?

I've been working on creatureQueue and ids. It'd be really handy to have a testing framework in place.

@DreadKnight
Copy link
Member Author

You mentioned you were going to talk to a friend. I don't want to jump the gun; would you accept a PR with Jest or another framework?

I've been working on creatureQueue and ids. It'd be really handy to have a testing framework in place.

I've noticed you poked at it and then you backed out of it, thought because build failed. Feel free to PR it again I guess.

@andretchen0
Copy link
Contributor

Great. Will do!

Yes, I should have used the "draft" PR button instead. I was having trouble with the automatic linter and TypeScript tests. I think it's ironed out.

@DreadKnight DreadKnight added abilities The issue is related to an unit ability or more and removed abilities The issue is related to an unit ability or more brainstorm Still need to figure out details about this labels Apr 10, 2023
DreadKnight added a commit that referenced this issue Apr 10, 2023
@DreadKnight
Copy link
Member Author

DreadKnight commented Apr 10, 2023

Alright, out of brainstorm as we went with Jest framework. Issue will be closed once we have some unit ability tests in.

DreadKnight added a commit that referenced this issue Apr 10, 2023
DreadKnight added a commit that referenced this issue Apr 11, 2023
CyberBishop pushed a commit to CyberBishop/AncientBeast that referenced this issue Apr 20, 2023
CyberBishop pushed a commit to CyberBishop/AncientBeast that referenced this issue Apr 20, 2023
CyberBishop pushed a commit to CyberBishop/AncientBeast that referenced this issue Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abilities The issue is related to an unit ability or more coding This issue requires some programming pipeline Affects how the project is being developed
Projects
None yet
Development

No branches or pull requests

3 participants