Unit test for VisionSharing component.
ClosedPublic
Actions

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

Details

Reviewers

mimo
Itms

Commits

rP19258: Unit test for VisionSharing component.

Summary

Following D117

Test Plan

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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

(Fix a typo in a comment)

elexis mentioned this in D117: Add spying to the game.Feb 27 2017, 10:16 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.

Harbormaster completed remote builds in B612: Diff 628.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/420/ for more details.

Harbormaster completed remote builds in B613: Diff 629.Feb 28 2017, 1:03 AM

(Do we need also acceptation for tests?)

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 ↗	(On Diff #629)	(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 ↗	(On Diff #629)	Nice trick to check Maps
156 ↗	(On Diff #629)	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 ↗	(On Diff #629)	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

Harbormaster completed remote builds in B646: Diff 666.Mar 1 2017, 10:08 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/443/ for more details.

Harbormaster completed remote builds in B646: Diff 666.Mar 1 2017, 11:34 PM

This looks good! Thanks for writing it ?

This revision is now accepted and ready to land.Mar 2 2017, 9:46 AM

Closed by commit rP19258: Unit test for VisionSharing component. (authored by fatherbushido). · Explain WhyMar 2 2017, 10:13 AM

This revision was automatically updated to reflect the committed changes.