yandex-ui / noscript

Noscript: JavaScript MVC Framework for building SPA
http://yandex-ui.github.io/noscript/
MIT License
34 stars 22 forks source link

Вынести логику ограничения перезапросов из canRequest #627

Closed vitkarpov closed 7 years ago

vitkarpov commented 7 years ago

https://github.com/yandex-ui/noscript/issues/625#issuecomment-241718597

626

vitkarpov commented 7 years ago

@alexeyten @chestozo

alexeyten commented 7 years ago

:+1:

chestozo commented 7 years ago

Может тесты canRequest переделать в тесты на _canRequest и дописать новых для canRequest()?

vitkarpov commented 7 years ago

Ага, тесты нужно поправить.

vitkarpov commented 7 years ago

Вообще, посмотрел сейчас тесты и, кажется, что логически все верно:

Все это остается верным и сейчас, и то, что поменялась внутренняя реализация (появился _canRequest + canRequest) не делает тесты на высокоуровневую логику неверными.

Имеет смысл добавить один тест — если мы определили canRequest явно в no.true, это не должно сломать логику с перезапросами. Вот в предыдущем варианте такой тест должен падать.

Добавлю его.

vitkarpov commented 7 years ago

https://github.com/yandex-ui/noscript/pull/629

chestozo commented 7 years ago

Я бы добавил ещё тест на то, что canRequest() => true / false и что от этого зависит результат вызова _canRequest() для полноты.

vitkarpov commented 7 years ago

тест на то, что canRequest() => true / false

такой тест уже есть https://github.com/yandex-ui/noscript/blob/master/test/spec/ns.request.js#L382-L449

chestozo commented 7 years ago

Оу ес ) Тогда 👍

vitkarpov commented 7 years ago

Тогда поправлю по комментариям и черипикну этот тест в свою веточку, а этот пул закрою. Там соответственно тест должен проходить :)

chestozo commented 7 years ago

кул ;)

vitkarpov commented 7 years ago

@chestozo готово. перенес тест + поправил по комментариям (sinon.timer не вкручивал — давай обсудим :)

chestozo commented 7 years ago

very 👍