simulation tests: eslint fixes for no-use-before-define
ClosedPublic
Actions

Authored by Krinkle on Jun 15 2019, 11:34 PM.

Details

Reviewers

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Owners Package	(Owns No Changed Paths)

Commits

rP22380: Define variables before mentioning them in scripted component tests, and add a…

Summary

There are only a handful of violations of the "latedef: nofunc" rule from the jshint in the
mods/public/simulation/components/tests directory. These are fixed in this commit, and
I also migrated the rule to eslint.

The warnings found weren't actual problems in this case see. The closures in which they were
references weren't called until later, so it all worked out.

Fixes the following warnings:

/0ad/binaries/data/mods/public/simulation/components/tests/test_Foundation.js
31:24  error  'pos' was used before it was defined            no-use-before-define
32:24  error  'pos' was used before it was defined            no-use-before-define

/0ad/binaries/data/mods/public/simulation/components/tests/test_Barter.js
43:26  error  'cmpBarter' was used before it was defined  no-use-before-define
44:32  error  'cmpBarter' was used before it was defined  no-use-before-define

Test Plan

I don't know the conventional way for 0ad developers to run linters locally, but I did it as follows:

Run a simple docker container with 0ad/ mounted and Node 10 installed.
Use npm to install eslint@5.16.0 and eslint-plugin-brace-rules@0.1.6 (current latest).
Run node_modules/.bin/eslint --cache -c build/jenkins/lint-config/eslintrc.json binaries/data/mods/public/simulation/components/tests/

Diff Detail

Repository

rP 0 A.D. Public Repository

Branch

/ps/trunk

Lint

Lint OK

Unit

No Unit Test Coverage

Build Status

Buildable 7959
Build 12954: arc lint + arc unit

Event Timeline

Krinkle created this revision.Jun 15 2019, 11:34 PM

Owners added a subscriber: Restricted Owners Package.Jun 15 2019, 11:34 PM

Harbormaster completed remote builds in B7959: Diff 8499.Jun 15 2019, 11:34 PM

Krinkle edited the test plan for this revision. (Show Details)Jun 15 2019, 11:35 PM

Stan awarded a token.Jun 15 2019, 11:54 PM

Stan added a reviewer: Restricted Owners Package.

Stan added a subscriber: Stan.Jun 16 2019, 5:14 PM

Stan added inline comments.

binaries/data/mods/public/simulation/components/tests/test_Foundation.js
123	You can move the initialization as well.

Krinkle added inline comments.Jun 16 2019, 10:15 PM

binaries/data/mods/public/simulation/components/tests/test_Foundation.js
123	Okay! I wasn't sure whether that safe (e.g. maybe some of mock set ups in-between are triggered indirectly from here) or preferred (maybe it's more readable in this location of the code).

Krinkle added inline comments.Jun 16 2019, 10:17 PM

binaries/data/mods/public/simulation/components/tests/test_Foundation.js
124	Should I keep this line where it is? I'm actually unsure how to run the JS tests locally. A pointer about that would help as well :)

Stan added inline comments.Jun 16 2019, 10:25 PM

binaries/data/mods/public/simulation/components/tests/test_Foundation.js
124	It's up to you :). But yeah the definition location doesn't change much as long as it's not called before being defined. you can find more about what construct component does in the file setup.js :)