hammerlab / cycledash

Variant Caller Analysis Dashboard and Data Management System
Other
35 stars 2 forks source link

Sanity-check and debug pdifftests #826

Closed armish closed 9 years ago

armish commented 9 years ago

This reverts the changes introduced as part of #814 into the update-screenshots.sh. Note that there are no changes to the screenshots and this passes the pdfifftests without any problem.

However, if you are to check this code out and run scripts/update-screenshots.sh, you do get different screenshots:

$ git branch |grep "*"
* sanity-check-pdiffs

$ git status
On branch sanity-check-pdiffs
Your branch is up-to-date with 'origin/sanity-check-pdiffs'.
nothing to commit, working directory clean

$ bash scripts/update-screenshots.sh 
Creating test DB, logging to tests/pdifftests/logs/DB_LOGs.txt...done.
Starting test server, logging to tests/pdifftests/logs/TEST_SERVER_LOGS.txt...done.
Opening SauceConnect tunnel from localhost..........................done.
Updating images...
 for Examine
  ✗ examine_bad-query: screenshots differ, updating
  ✗ examine_base: screenshots differ, updating
 ...

which is unexpected as these new screenshots don't match with the ones generated via Travis.

Review on Reviewable

armish commented 9 years ago

@ihodes: never mind the earlier differing screenshots. I think they were temporary as 9 out of 10 trials, the update screenshots scripts run OK:

$ bash scripts/update-screenshots.sh 
Creating test DB, logging to tests/pdifftests/logs/DB_LOGs.txt...done.
Starting test server, logging to tests/pdifftests/logs/TEST_SERVER_LOGS.txt...done.
SauceConnect tunnel detected already running, will attempt to use that.
Updating images...
 for Examine
  ✓ examine_bad-query: no change
  ✓ examine_base: no change
  ✓ examine_comments-edit: no change
  ✓ examine_comments-view: no change
  ✓ examine_filter: no change
  ✓ examine_sorted: no change
  ✓ examine_tooltip: no change
  ✓ examine_validation: no change
 for Runs
  ✓ runs_add-bam: no change
  ✓ runs_add-run: no change
  ✓ runs_bams: no change
  ✓ runs_info: no change
  ✓ runs_new-project: no change
  ✓ runs_page: no change
 for Tasks
  ✓ tasks_page: no change
 for Website
  ✓ website_about-page: no change
  ✓ website_comments: no change
  ✓ website_login: no change
  ✓ website_register: no change

Seems you were right: we didn't need that change in this script, but a version bump for Chrome was good enough to fix the problem.

ihodes commented 9 years ago

Crazy. Thanks for tracking this down!