Fix up broken js-tests.

Review Request #10291 — Created Oct. 29, 2018 and submitted

Information

Review Board
release-4.0.x
3c25ac3...

Reviewers

We had a few JavaScript unit tests which were broken, with two specific
causes:

  • For the ScrollManagerView, chrome has been doing sub-pixel
    scrolling, which broke our tests. I've sprinkled some Math.round()
    calls around to fix this.
  • For the ReviewReplyEditorModel, an update to Backbone had broken
    some tests which artificially assign an ID in order for isNew() to
    return false. The newer isNew() checks the id attribute
    instead of the value in the object.

Ran js-tests.

Description From Last Updated

If we call set('id', ...) instead, this will take care of both. We should be doing that for IDs going …

chipx86chipx86

Same here.

chipx86chipx86

I'm not sure it's correct to round it here. I can see rounding for the tests, but I think interfering …

chipx86chipx86
chipx86
  1. 
      
  2. Show all issues

    If we call set('id', ...) instead, this will take care of both. We should be doing that for IDs going forward.

    1. Latest change doesn't have this fix.

  3. Show all issues

    I'm not sure it's correct to round it here. I can see rounding for the tests, but I think interfering with original values here will throw off calculations. Or if we need to round, doing it when applying the new location might be better.

  4. 
      
david
david
chipx86
  1. Ship It!
  2. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-4.0.x (198956c)