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

Create some unit tests #7

Closed
jhofman opened this issue Apr 5, 2021 · 9 comments · Fixed by #122
Closed

Create some unit tests #7

jhofman opened this issue Apr 5, 2021 · 9 comments · Fixed by #122
Assignees

Comments

@jhofman
Copy link
Contributor

jhofman commented Apr 5, 2021

This will require some careful thought because the output are visualizations, not numerical results.

Maybe comparing to a snapshot or checksum or something?

And perhaps hook this up to Github actions for continuous integration?

@jhofman
Copy link
Contributor Author

jhofman commented Nov 16, 2021

Looping @chisingh in on this, let's coordinate on unit tests for R and python.

One idea that wasn't obvious when this was created, but makes sense now: unit tests can check the specs generated on the backend language side, perhaps not a checksum or hash but checking for certain key fields in the generated json.

@jhofman
Copy link
Contributor Author

jhofman commented Nov 18, 2021

It looks like there are some old specs that no longer work, e.g. here:

https://github.com/microsoft/datamations/blob/main/inst/htmlwidgets/test/index.html

Let's get rid of them to avoid confusion going forward.

@sharlagelfand
Copy link
Collaborator

I've cleaned out the sandbox folder so it only has working specs.

@sharlagelfand
Copy link
Collaborator

sharlagelfand commented Nov 18, 2021

Re: testing, there are a bunch of tests here that check the format / fields of the json on the R end. Definitely not 100% test coverage because of all the shiny, legacy code, etc, but a decent start on the core functionality:

Screen Shot 2021-11-18 at 4 04 47 PM

(the middle columns show lines, covered, missed, etc - but the last column is coverage for each file)

I'm still trying to think of the best way to compare the python vs R specs within the testthat framework (which is commonly used for R packages). If the python code can write specs to a json file, we can read those in, generate the R specs, and check that they're the same as a start!

@jhofman
Copy link
Contributor Author

jhofman commented Nov 23, 2021

sounds like a good idea on writing python json specs out to files and testing them w/ the R testthat framework.

@chisingh, let's work on this once python is up and running.

@chisingh
Copy link
Contributor

I have added unit tests using pytest framework that can also be enabled to run after every commit once issue #104 is closed. Some of these tests use the raw json files provided by @sharlagelfand to verify that both the outputs match.

@jhofman
Copy link
Contributor Author

jhofman commented Nov 30, 2021

@chisingh will output json specs from python and @sharlagelfand will read that in and test w/ testthat in R

@sharlagelfand
Copy link
Collaborator

Just working on testing those specs @chisingh and it looks like there are some discrepancies in ordering of things - in the R specs, meta comes before data, but in the python specs data comes first.

Similarly in the data.values, the order of fields do not match, e.g. in R:

"gemini_id": 20,
"Work": "Academia",
"datamations_x": 1,
"datamations_y": 85.0122219615483,
"datamations_y_tooltip": 85.0122219615483,
"datamations_y_raw": 85.3303201349918,
"Lower": 84.7637160761228,
"Upper": 85.2607278469738

versus in python:

"gemini_id": 20,
"Work": "Academia",
"datamations_x": 1,
"datamations_y": 85.01222196154829,
"datamations_y_raw": 85.33032013499178,
"datamations_y_tooltip": 85.01222196154829,
"Lower": 84.76371607612279,
"Upper": 85.2607278469738

The order of datamations_y_raw and datamations_y_tooltip are flipped here.

Otherwise the specs look equivalent! Just something it might be nice to keep in mind and change if not too big of a hassle, otherwise I will have to match the ordering before each test.

@chisingh
Copy link
Contributor

chisingh commented Dec 3, 2021

That's great! I will reorder those fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants