Skip to content

Fixes LMS-2474 by ensuring that readElts reads always reads an element - #3134

Merged
davestgermain merged 1 commit into
masterfrom
dcs/a11y-gentle-alert
Apr 1, 2014
Merged

Fixes LMS-2474 by ensuring that readElts reads always reads an element#3134
davestgermain merged 1 commit into
masterfrom
dcs/a11y-gentle-alert

Conversation

@davestgermain

Copy link
Copy Markdown
Contributor

@adampalay

Copy link
Copy Markdown
Contributor

@davestgermain , running the new test with the old code in capa/display.coffee doesn't yield a test failure. It should...

@davestgermain

Copy link
Copy Markdown
Contributor Author

@adampalay oh, duh... I wasn't testing the right thing. OK, I figured out how to load the SR object in the tests, and verified that the old code failed the test.

@adampalay

Copy link
Copy Markdown
Contributor

this needs a rebase too

@adampalay

Copy link
Copy Markdown
Contributor

👍 , but it should get another review.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@davestgermain For consistency's sake (see how spyOn is used throughout this file), please update to:

spyOn SR, 'readElts'
spyOn SR, 'readText'

@valera-rozuvan

Copy link
Copy Markdown
Contributor

@davestgermain Code looks good. Waiting to see what Jenkins says.

@valera-rozuvan

Copy link
Copy Markdown
Contributor

@davestgermain 👍 Good to merge!

davestgermain pushed a commit that referenced this pull request Apr 1, 2014
Fixes LMS-2474 by ensuring that readElts reads always reads an element
@davestgermain
davestgermain merged commit 66ecbc9 into master Apr 1, 2014
@davestgermain
davestgermain deleted the dcs/a11y-gentle-alert branch April 1, 2014 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants