search

DECIPHER-genomics / Genoverse

HTML5 scrollable genome browser
https://genoverse.org/
Other
108 stars 44 forks source link

Fix broken Scaleline, Legend and Labels track tests #64

Closed tomratcliffe closed 3 years ago

tomratcliffe commented 3 years ago

The tests were failing because of slight pixel offsets being incorrect.

It would probably be easier to debug these if we were generating and writing to snapshot files, as we could then see the changes to be made to adjust the tests exactly.

As it stands, the tests probably contain a little too much logic, as they're reimplementing the drawing logic, but for now this is the best way to fix the tests by adjusting the draw coordinates

Example of the difference in previously failing tests (easiest to see the difference when opening both images and switching between tabs): genoversetest2 genoversetest1

And a more structural change:

Expected (generated from code in the tests): genoverseexpected

Actual: genoverseactual

tomratcliffe commented 3 years ago

(Just noticed that tests are passing locally but failing on a CircleCI build elsewhere, might be due to some other DOM differences when running on Mac vs Linux...)

simonbrent commented 3 years ago

I run the tests on Ubuntu 18.04, and they pass. With your changes, they fail. As such I will not be accepting this pull request.

I acknowledge, however, that the test suite is a pain to understand and work with. Snapshots may be a better approach, although a snapshot which contains a data URL only records the output of the track at the time it was created, rather than whether that output was actually what it was meant to be!

tomratcliffe commented 3 years ago

Hi @simonbrent, thanks - yup I was just investigating after opening this and couldn't see exactly why the differences in rendering were happening without going down a rabbit hole. I agree on closing this.

I noticed that the tests are failing on travis here though: https://travis-ci.org/github/wtsi-web/Genoverse/builds/765507940

It's the same Scaleline track - the other ones here were some that I noticed behaved differently on my machine, but they pass on Travis

simonbrent commented 3 years ago

Sigh...I failed to check Travis (actually, I routinely forget it exists, given how infrequently I actually commit to this repo)

simonbrent commented 3 years ago

@tomratcliffe I have just switched from Travis to Github actions, because I couldn't work out how to get node-canvas to work with Travis in Ubuntu 18.04.

While doing this I noticed that the tests fail on Ubuntu 18.04 with node 12, but pass with node 14 and 15. I recommend trying to get your CI to use node 14/15 (npm install failed for node-canvas on node 16, but given how new that is I expect this to be fixed at some point).