Following D117
Details
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 Build Jenkins Build 970: arc lint + arc unit
Event Timeline
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.
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? |
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. |
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.