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

[DeckGL] Added fixtures and Deck test #4798

Merged
merged 7 commits into from
Apr 13, 2018
Merged

[DeckGL] Added fixtures and Deck test #4798

merged 7 commits into from
Apr 13, 2018

Conversation

hughhhh
Copy link
Member

@hughhhh hughhhh commented Apr 10, 2018

Add utils.py for better testing examples moving that can be loaded from a json

@betodealmeida @mistercrunch

@codecov-io
Copy link

codecov-io commented Apr 11, 2018

Codecov Report

Merging #4798 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4798      +/-   ##
==========================================
+ Coverage   72.38%   72.46%   +0.08%     
==========================================
  Files         208      208              
  Lines       15526    15530       +4     
  Branches     1203     1203              
==========================================
+ Hits        11238    11254      +16     
+ Misses       4285     4273      -12     
  Partials        3        3
Impacted Files Coverage Δ
superset/viz.py 80.35% <100%> (+0.84%) ⬆️
...set/assets/javascripts/explore/stores/controls.jsx 38.88% <0%> (+0.86%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dadc057...9c6b8c3. Read the comment docs.

tests/utils.py Outdated

import json

FIXTURES_DIR = 'tests/fixtures'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit fragile, and might fail depending on from which directory you run the unit tests. There's a more robust way of handling this:

  1. Make sure the json files are in MANIFEST.in.
  2. Make sure setup.py has the line include_package_data=True (it has).
  3. You can now read the json content from using pkg_resources.resource_filename no matter where it was installed.

For 1 & 2, see this reference. For 3, see this reference.

Copy link
Member

@mistercrunch mistercrunch Apr 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also just have the fixtures be defined as python data structures and json.dumps them if/when needed. Personally I rarely write JSON as I don't like the fact it doesn't support trailing commas and triple quoted strings.

@mistercrunch mistercrunch merged commit 2f5cff7 into apache:master Apr 13, 2018
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
* added fixtures and deck test

* linting

* linting

* add os.path

* remove para

* fix reference
timifasubaa pushed a commit to timifasubaa/incubator-superset that referenced this pull request May 31, 2018
* added fixtures and deck test

* linting

* linting

* add os.path

* remove para

* fix reference
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
* added fixtures and deck test

* linting

* linting

* add os.path

* remove para

* fix reference
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.25.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.25.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants