Page MenuHomeWildfire Games

Unit test for VisionSharing component.
ClosedPublic

Authored by fatherbushido on Feb 27 2017, 7:48 PM.

Details

Summary

Following D117

Test Plan

-

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 613
Build 971: Vulcan BuildJenkins
Build 970: arc lint + arc unit

Event Timeline

fatherbushido created this revision.Feb 27 2017, 7:48 PM

(Fix a typo in a comment)

Vulcan added a subscriber: Vulcan.Feb 27 2017, 11:24 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/419/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/420/ for more details.

(Do we need also acceptation for tests?)

elexis added a subscriber: elexis.Mar 1 2017, 11:10 AM

Looks good. Besides OnOwnershipChanged and OnGarrisonedUnitsChanged, all functions in the component are tested, which should be sufficient for a test.
Don't forget the eol-style native attribute when committing. I can post an acceptance if you want and if the reviewers don't have any (further) comments.

binaries/data/mods/public/simulation/components/tests/test_VisionSharing.js
83

(In most occurances when functions are assigned to a variable, I see the opening brace on the same line as the function declaration, but it's not settled officially and probably inconsistent with the convention even)

134

Nice trick to check Maps

156

Where's that 48 coming from? Arbitrary right? Then we could remove it too?

fatherbushido added inline comments.Mar 1 2017, 9:19 PM
binaries/data/mods/public/simulation/components/tests/test_VisionSharing.js
156

It's arbitrary. The one in the mock above. I just choose a signifactive number to know what it is :/ And I was lazy to use a variable.

native eol-style

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/443/ for more details.

Itms accepted this revision.Mar 2 2017, 9:45 AM
Itms awarded a token.

This looks good! Thanks for writing it ?

This revision is now accepted and ready to land.Mar 2 2017, 9:46 AM
This revision was automatically updated to reflect the committed changes.