Page MenuHomeWildfire Games
Differential D772

Unit test for DeathDamage
ClosedPublic
Actions

Authored by fatherbushido on Aug 8 2017, 10:38 AM.

Details

Reviewers
None
Commits
rP19954: Add unit tests for DeathDamage component. Refs rP19950.
Summary

refs D451

Test Plan

./binaries/system/test

Diff Detail

Event Timeline

fatherbushido created this revision.Aug 8 2017, 10:38 AM
Stan requested changes to this revision.Aug 8 2017, 10:48 AM
Stan added a subscriber: Stan.
Stan added inline comments.
binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
13

Ternary maybe ?

28

Damages → Damage unless you are talking about “money claimed by, or ordered to be paid to, a person as compensation for loss or injury” source https://blog.harwardcommunications.com/2011/02/21/the-difference-between-damage-and-damages/

40

It's not mandatory but I think it's good to follow the json notation and add quotes around properties so that CauseSplashDamage: becomes → "CauseSplashDamage:" same for the other lines.

This revision now requires changes to proceed.Aug 8 2017, 10:48 AM
fatherbushido added inline comments.Aug 8 2017, 10:54 AM
binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
13

no

28

thx, I would have trusted you without source.

40

yes

fatherbushido updated this revision to Diff 3032.Aug 8 2017, 10:56 AM
fatherbushido edited edge metadata.

cosmetic

Harbormaster completed remote builds in B2718: Diff 3032.Aug 8 2017, 10:56 AM
Vulcan added a subscriber: Vulcan.Aug 8 2017, 11:02 AM
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  40| »   GetPlayersToDamage:·(owner,·friendlyFire)·=>·playersToDamage
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'owner' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  11| »   »   return·stat·+·200
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  13| }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  15| let·deadEnt·=·60;
|    | [NORMAL] JSHintBear:
|    | 'deadEnt' was used before it was defined.

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  25| }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  35| let·playersToDamage·=·[2,·3,·7]
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  52| let·result·=·{
|    | [NORMAL] JSHintBear:
|    | 'result' was used before it was defined.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/356/ for more details.

elexis added a subscriber: elexis.Aug 8 2017, 11:06 AM
elexis added inline comments.
binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
61

-1 tab
Perhaps that should be defined before usage (or linlined) for better readability. (iirc hoisting let-defined variables will break on SM45)
(moving modifiedDamage may makes sense too, dunno)

fatherbushido updated this revision to Diff 3033.Aug 8 2017, 11:10 AM

I was waiting for Lint bot

Harbormaster completed remote builds in B2719: Diff 3033.Aug 8 2017, 11:10 AM
fatherbushido updated this revision to Diff 3034.Aug 8 2017, 11:11 AM

tabs (noticed by elexis)

Stan added a comment.Aug 8 2017, 11:18 AM

Maybe we should check that surrounding entities actually get damaged ? (How much damage they get should be tested in damage.js though)
Also I'm not sure on how you could test that a warning is thrown if there is no owner (because apparently that's not a problem).

fatherbushido removed a reviewer: Stan.EditedAug 8 2017, 11:29 AM
fatherbushido added inline comments.Aug 8 2017, 11:33 AM
binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
61

yep, bot noticed it too

Vulcan added a comment.Aug 8 2017, 11:50 AM
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  40| »   "GetPlayersToDamage":·(owner,·friendlyFire)·=>·playersToDamage
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'owner' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  11| »   »   return·stat·+·200
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  13| }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  15| let·deadEnt·=·60;
|    | [NORMAL] JSHintBear:
|    | 'deadEnt' was used before it was defined.

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  25| }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  35| let·playersToDamage·=·[2,·3,·7]
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  52| let·result·=·{
|    | [NORMAL] JSHintBear:
|    | 'result' was used before it was defined.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/357/ for more details.

Stan added a comment.Aug 8 2017, 12:31 PM
In D772#30436, @fatherbushido wrote:

Uh ? did I offend you somehow ?

fatherbushido added a comment.Aug 8 2017, 12:34 PM
In D772#30458, @Stan wrote:
In D772#30436, @fatherbushido wrote:

Uh ? did I offend you somehow ?

Not at all.

Vulcan added a comment.Aug 8 2017, 12:37 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

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

Harbormaster completed remote builds in B2717: Diff 3031.Aug 8 2017, 12:37 PM
Vulcan added a comment.Aug 8 2017, 12:38 PM
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  16| }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/358/ for more details.

fatherbushido updated this revision to Diff 3035.Aug 8 2017, 12:40 PM

Last semicolon. Noticed by Vulcan

Imarok added a subscriber: Imarok.Aug 8 2017, 12:53 PM

Extra lint:

Executing section JS...
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /media/LiExtend/0ADPhabsvn/binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|    |++++| /media/LiExtend/0ADPhabsvn/binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  33|  33| let cmpDeathDamage = ConstructComponent(deadEnt, "DeathDamage", template);
|  34|  34| 
|  35|  35| let playersToDamage = [2, 3, 7];
|  36|    |-let pos = new Vector2D(3,4.2);
|    |  36|+let pos = new Vector2D(3, 4.2);
|  37|  37| 
|  38|  38| let result = {
|  39|  39| 	"attacker": deadEnt,
fatherbushido added a comment.Aug 8 2017, 1:08 PM
In D772#30469, @Imarok wrote:

Extra lint:

Executing section JS...
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /media/LiExtend/0ADPhabsvn/binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|    |++++| /media/LiExtend/0ADPhabsvn/binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  33|  33| let cmpDeathDamage = ConstructComponent(deadEnt, "DeathDamage", template);
|  34|  34| 
|  35|  35| let playersToDamage = [2, 3, 7];
|  36|    |-let pos = new Vector2D(3,4.2);
|    |  36|+let pos = new Vector2D(3, 4.2);
|  37|  37| 
|  38|  38| let result = {
|  39|  39| 	"attacker": deadEnt,

How can you do that? Learn me to fish please.

fatherbushido updated this revision to Diff 3036.Aug 8 2017, 1:09 PM

Missing space (noticed by Imarok and its extra linter).

Imarok added a comment.Aug 8 2017, 1:12 PM
In D772#30471, @fatherbushido wrote:
In D772#30469, @Imarok wrote:

Extra lint:

Executing section JS...
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /media/LiExtend/0ADPhabsvn/binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|    |++++| /media/LiExtend/0ADPhabsvn/binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
|  33|  33| let cmpDeathDamage = ConstructComponent(deadEnt, "DeathDamage", template);
|  34|  34| 
|  35|  35| let playersToDamage = [2, 3, 7];
|  36|    |-let pos = new Vector2D(3,4.2);
|    |  36|+let pos = new Vector2D(3, 4.2);
|  37|  37| 
|  38|  38| let result = {
|  39|  39| 	"attacker": deadEnt,

How can you do that? Learn me to fish please.

I just installed the linter on my local machine and added some more options which I proposed in D213 (and which will hopefully applied to the public linter soon) ;P

Closed by commit rP19954: Add unit tests for DeathDamage component. Refs rP19950. (authored by fatherbushido). · Explain WhyAug 8 2017, 1:13 PM
This revision was automatically updated to reflect the committed changes.
fatherbushido added a comment.Aug 8 2017, 1:15 PM
In D772#30482, @Imarok wrote:

I just installed the linter on my local machine and added some more options which I proposed in D213 (and which will hopefully applied to the public linter soon) ;P

Looks like an inception, now I want that to be commited :)

Imarok added a comment.Aug 8 2017, 1:17 PM
In D772#30486, @fatherbushido wrote:
In D772#30482, @Imarok wrote:

I just installed the linter on my local machine and added some more options which I proposed in D213 (and which will hopefully applied to the public linter soon) ;P

Looks like an inception, now I want that to be commited :)

Me too ;)
(I hope Itms will care, as soon as he is back)

Vulcan added a comment.Aug 8 2017, 1:26 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

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

Harbormaster completed remote builds in B2718: Diff 3032.Aug 8 2017, 1:26 PM
Vulcan added a comment.Aug 8 2017, 1:27 PM
Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/359/ for more details.

Vulcan added a comment.Aug 8 2017, 3:04 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

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

Harbormaster completed remote builds in B2719: Diff 3033.Aug 8 2017, 3:04 PM
Vulcan added a comment.Aug 8 2017, 3:05 PM
Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/360/ for more details.

Vulcan added a comment.Aug 8 2017, 4:04 PM

Build has FAILED

Link to build: http://jw:8080/job/phabricator/1812/
See console output for more information: http://jw:8080/job/phabricator/1812/console

Harbormaster failed remote builds in B2720: Diff 3034!Aug 8 2017, 4:04 PM
Vulcan added a comment.Aug 8 2017, 4:05 PM
Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/361/ for more details.

Vulcan added a comment.Aug 8 2017, 4:52 PM

Build has FAILED

Link to build: http://jw:8080/job/phabricator/1813/
See console output for more information: http://jw:8080/job/phabricator/1813/console

Harbormaster failed remote builds in B2721: Diff 3035!Aug 8 2017, 4:53 PM
Vulcan added a comment.Aug 8 2017, 5:50 PM

Build has FAILED

Link to build: http://jw:8080/job/phabricator/1814/
See console output for more information: http://jw:8080/job/phabricator/1814/console

Harbormaster failed remote builds in B2722: Diff 3036!Aug 8 2017, 5:50 PM

Revision Contents
Changeset List

PathSize
binaries/
data/
mods/
public/
simulation/
components/
tests/
64 lines

Diff 3035

View Options

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js

Loading...
Wildfire Games · Phabricator