Skip to content

Commit

Permalink
Fix bugs in RecordStore that result in race conditions being met in
Browse files Browse the repository at this point in the history
pdiff tests.

(Issues being filed for other possible race conditions that don't appear
in tests, but are theoretically possible; RecordStore needs a bit of
rework)
  • Loading branch information
ihodes committed Mar 24, 2015
1 parent b71ebf8 commit 53eeb84
Show file tree
Hide file tree
Showing 16 changed files with 23 additions and 36 deletions.
18 changes: 0 additions & 18 deletions DEVELOP.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,24 +161,6 @@ e.g. `./tests/pdifftests/run.sh -d chrome` to run with the Chrome driver, or
`./tests/pdifftests/run.sh -c examine -f base` to filter the tests to those in
the 'examine' class with 'base' in their name.

Depnding on your version of firefox, you may find it useful to specify the path
to the binary you'd like the tests to run on with `--firefox-path` (you can also
set this in `.seltestrc`, see
[the github page for seltest](https://github.com/ihodes/seltest)).

Our tests are generated with Firefox
[release 34](https://ftp.mozilla.org/pub/mozilla.org/firefox/releases/34.0/), on
OSX. You can download this, save it somewhere (maybe in your virtualenv folder),
and then point `seltest` to it like so, in `~/.seltestrc`:

```
[arguments]
--firefox-path=~/directory/to/Firefox.app/Contents/MacOS/firefox-bin
```

Be sure to turn off auto-update in Firefox preferences if you plan on opening
it. Seltest will automatically disable updates when running tests.

To determine whether there are any pixels that have changed before/after, and to
generate a perceptual diff that will make it clear where the changes are, you
can use [webdiff](https://github.com/danvk/webdiff): `git webdiff`.
12 changes: 3 additions & 9 deletions cycledash/static/js/examine/RecordStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,15 @@ function createRecordStore(run, igvHttpfsUrl, dispatcher, opt_testDataSource) {
// Required: lets the dispatcher know that the Store is done processing.
return true;
}
if (dispatcher) dispatcherToken = dispatcher.register(receiver);
if (dispatcher) dispatcherToken = dispatcher.register(receiver);

/**
* Queries the backend for the set of genotypes matching the current
* parameters.
*
* NB: mutates store state!
*/
function _updateGenotypes({append}) {
function updateGenotypes({append}) {
// Example query:
// {"range": {"contig": 1, "start": 800000, "end": 2000000},
// "sortBy": [{"columnName": "sample:DP", "order": "desc"},
Expand Down Expand Up @@ -154,9 +154,6 @@ function createRecordStore(run, igvHttpfsUrl, dispatcher, opt_testDataSource) {
notifyChange(); // notify of pending request
deferredGenotypes(vcfId, query)
.done(response => {
if (!_.isEqual(currentPendingQuery, query)) {
return; // A subsequent request has superceded this one.
}
if (append) {
_.extend(keyToRecordIndex,
generateKeyToRecordIndex(response.records, records.length));
Expand All @@ -180,8 +177,6 @@ function createRecordStore(run, igvHttpfsUrl, dispatcher, opt_testDataSource) {
notifyChange();
});
}
var updateGenotypes =
_.debounce(_.throttle(_updateGenotypes, 500 /* ms */), 500 /* ms */);

// Ignore all currently pending requests (presumably because there's a newer one).
function ignorePendingRequests() {
Expand Down Expand Up @@ -426,8 +421,7 @@ function createRecordStore(run, igvHttpfsUrl, dispatcher, opt_testDataSource) {
}
}

// There's no need to debounce this update -- make it so now!
_updateGenotypes({append: false});
updateGenotypes({append: false});
getComments();

function notifyChange() {
Expand Down
11 changes: 6 additions & 5 deletions tests/pdifftests/README.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
Notes on working with image diff tests
--------------------------------------
Notes on working with perceptual diff tests
-------------------------------------------

There are some notes in [DEVELOP.md](/DEVELOP.md) about how to run the dpxdt tests to
check for changes. This document describes how to update and extend them.
There are some notes in [DEVELOP.md](/DEVELOP.md) about how to run the
perceptual diff tests to check for changes. This document describes how to
update and extend them.

CycleDash requires a PostgreSQL database to run, and a rabbitmq workqueue to
process new inputs. For the tests to be reproducible, it needs its own
instances of each of these.

You must have postgres running to update the dpxdt tests, e.g. by running:
You must have Postgres running to update the tests, e.g. by running:

postgres -D /usr/local/var/postgres

Expand Down
Binary file modified tests/pdifftests/images/examine_bad-query.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/pdifftests/images/examine_base.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/pdifftests/images/examine_comments-edit.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/pdifftests/images/examine_comments-view.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/pdifftests/images/examine_filter.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/pdifftests/images/examine_sorted.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/pdifftests/images/examine_tooltip.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/pdifftests/images/examine_validation.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/pdifftests/images/runs_info.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/pdifftests/images/runs_page.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/pdifftests/images/website_comments.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
11 changes: 9 additions & 2 deletions tests/pdifftests/run.sh
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
#!/usr/bin/env sh
#!/bin/bash

source tests/ENV.sh

python run.py > tests/pdifftests/log.txt 2>&1 &
RUN_PID=$!
echo pid of test server is $RUN_PID
echo logging to tests/pdifftests/log.txt

function finish {
kill $RUN_PID
}
trap finish EXIT

sel update -b phantomjs -o tests/pdifftests/images "$@" tests/pdifftests
kill $RUN_PID
7 changes: 5 additions & 2 deletions tests/pdifftests/test.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# -*- coding: utf-8 -*-
from seltest import url, waitfor, Base
from seltest import url, waitfor, dontwaitfor, Base
from selenium.webdriver.support.ui import Select
from selenium.webdriver.common.action_chains import ActionChains

Expand Down Expand Up @@ -45,6 +45,7 @@ def bams(self, driver):
class Examine(Base):
window_size = [1280, 800]
base_url = BASE + '/runs/1/examine'
wait_for = {'css_selector': '.query-status', 'classes': ['good']}

def base(self, driver):
"""Initial view of a fully-loaded Examine page."""
Expand All @@ -59,7 +60,7 @@ def sorted(self, driver):
@waitfor('[data-attribute="info:DP"] .tooltip')
def tooltip(self, driver):
"""Examine page showing a Normal Read Depth tooltip."""
dp = driver.find_element_by_css_selector('[data-attribute="info:DP"]')
dp = driver.find_element_by_css_selector('[data-attribute="sample:RD"]')
hover = ActionChains(driver).move_to_element(dp)
hover.perform()

Expand All @@ -81,6 +82,8 @@ def comments_edit(self, driver):
assert btn.text.lower() == 'edit'
btn.click()

@dontwaitfor('.query-status')
@waitfor('.query-status', classes=['bad'])
def bad_query(self, driver):
"""Examine page showing a poorly formed query."""
input = driver.find_element_by_css_selector('input[type="text"].tt-input')
Expand Down

0 comments on commit 53eeb84

Please sign in to comment.