Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Js tests #114

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Js tests #114

wants to merge 10 commits into from

Conversation

rebfrank
Copy link
Contributor

I'm so sorry for the delay in sending this pr. I am planning on writing some more tests this afternoon but thought I'd better send an initial pr over now to hold myself accountable :)

I should have some time to make edits so please feel free to share any feedback.

cc @mscarey

Run npm test in js/ directory.

Right now the tests get the same Map object, because the require() always returns a reference to the
same object, even though it is called once per test. This is undesirable because it could cause
confusing interdependencies between tests.
- Add a new mock element in the dom for the year selector
- Modify mock data to reflect the new data format
I want to be able to isolate the map class to test it. The changes here are:
- Move the const mapping of data keys to user-facing strings inside a map class function. This also
  avoids needing to worry about updating the previously cached Map.populationKey and map.groupKey
  values (now it is updated on demand)
- Pass in the year selector to the Map class
- Modify index.js to export the Map class rather than an instance of the Map class
- Modify index.html to instantiate a Map now that index.js does not
- Style of district data that does not exist returns stripes
- Style of district that is missing data returns stripes
- Popup of district with higher count than population shows error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant