search

demipixel / factorio-blueprint

Import and Export blueprint strings automagically with this handy dandy blueprint API
MIT License
105 stars 29 forks source link

Multiple Blueprint unit tests. #34

Closed CandleCandle closed 6 years ago

CandleCandle commented 6 years ago

Add many unit tests prior to refactoring and #32.

Known Issues: implementation of constant-combiner behaviour and filter-inserter behaviour conflicts. Reverted some of the parseFilters implementation until there's a simple unit test for constant-combiners (I rebased my changes onto master and my filter-inserter test started failing!)

Two unit test frameworks: I used with mocha as converting one test to use mocha was easier than converting all of mine. Should pick one, and remove the other.

Lots of gaps that need simple blueprints from in-game and assertions adding.

demipixel commented 6 years ago

This is awesome! This has been on my todo list for this project for a while. I’ll have to look this over later. I’m curious, however, what did you add with respect to #32?

demipixel commented 6 years ago

Accidentally closed it, whoops :P

So, I noticed

assert.equal(obj.blueprint.entities[0].filters[0].index, 1); // TODO possible bug here; the parse test has it indexed from 0.

Not sure what the issue is? The array index (filters[i]) when handled by Factorio means nothing, it only looks at the index.

CandleCandle commented 6 years ago

with respect to #32, nothing directly, this is preparation before refactoring the entity types - I wanted a set of unit tests before refactoring to ensure that I haven't broken anything,

The possible bug is that lua tables are 1-indexed, and so re-indexing the filters means that there's a chance that the 0-index filter item is ignored when the blueprint is loaded; that was a TODO for me to check the behaviour,

demipixel commented 6 years ago

Do you think this PR is ready for merging?

CandleCandle commented 6 years ago

I think it can be. There are some stub tests (e.g. tests that need sample blueprints from in-game) and some tests that attempt to demonstrate features that are missing (e.g. train stop circuit conditions).

I think that in hindsight, it will be ready when the first problem is complete (stub tests). The second (missing features) should be much easier with (a) the set of tests and (b) the change to how entities are handled that I have a prototype for - I'll try to push that to a branch in my fork so that you can have a look at it; It's far from complete, and that branch is highly likely to have force-pushes to it though.

CandleCandle commented 6 years ago

Found some more missing features, added tests that can demonstrate them; filled in the remaining stub tests.

If you are happy with the changes, I think that now it is ready to merge.

demipixel commented 6 years ago

Thank you so, so much!