search

bem / html-differ

Сompares two HTML
http://bem.info/tools/testing/html-differ/
MIT License
211 stars 44 forks source link

Update 'ignoreComments' option #116

Closed eGavr closed 9 years ago

eGavr commented 9 years ago

The new realization of this option:

ignoreComments: {
    conditionalComments: false,
    normalComments: true
}
arikon commented 9 years ago

@j0tunn @eGavr Стоит сделать

eGavr commented 9 years ago

А зачем нужны комментарии?

alexbaumgertner commented 9 years ago

@eGavr чтобы не плодить файлы тестов, когда нужно проверить например разные модификаторы или разный проброс параметров в одинаковых блоках – можно в одном файле сделать несколько шаблонов /cc @Coltspb

alexbaumgertner commented 9 years ago

@j0tunn @eGavr @arikon очень стоит, для нас это Блокер

j0tunn commented 9 years ago

@alexbaumgertner почему это блокер? Тесты подправить - да и все, тем более, что у тебя там просто нужно свойства местами поменять

eGavr commented 9 years ago

Я вижу смысл в том, чтобы сейчас править тесты руками, а более умная проверка conditional comments появится вскоре.

sipayRT commented 9 years ago

чтобы не плодить файлы тестов, когда нужно проверить например разные модификаторы или разный проброс параметров в одинаковых блоках – можно в одном файле сделать несколько шаблонов

зачем вообще хотеть такого? Давайте тогда уже сразу все в один файл писать :) Есть один независимый тест, не нужно смешивать.

eGavr commented 9 years ago

Итак!

решили, что СС будут проверяться как строка, а внутреннее содержание - как тэги (с игнорированием последовательности атрибутов и т.д.)

Руководствуюсь этим алгоритмом. Все "ЗА"?

alexbaumgertner commented 9 years ago

@j0tunn поправил, все равно не проходит: 2015-02-03 17-43-20 update ignorecomments option issue 116 bem html-differ

ПС: убрал еще пробелов и стало проходить

eGavr commented 9 years ago

Дело в том, что эти комментарии на данном этапе сравниваются как обычный текст, поэтому пробелы и переносы строк будут учитваться вне зависимости от опции ignoreWhitespaces, так и было задумано по алгоритму, ведь по логике мы и должны так делать.

Кейс с CC я учту ASAP. Уж помучайтесь чуть-чуть и поудаляйте пробелы ;) Поправлю, и все будет как вам надо в скором времени.

Игнорировать комменты мы не можем, так как @sipayRT привел реальный пример баги.

sipayRT commented 9 years ago

Руководствуюсь этим алгоритмом. Все "ЗА"?

я ЗА

alexbaumgertner commented 9 years ago

@eGavr ок :) @eGavr @j0tunn @sipayRT спасибо за разъяснения и коментарии, поправлю в библиотеках.

sipayRT commented 9 years ago

image

persidskiy commented 9 years ago

Вы главное скажите, ручные комментарии можно будет писать в эталонных шаблонах?

eGavr commented 9 years ago

Нет.

persidskiy commented 9 years ago

Нет, так не пойдет, давай сделаем сразу хорошо.

Предвещая вопрос зачем мне комментарии и что я этого не должен хотеть, отвечу сразу. Я могу хотеть держать эталонные файлы в опрятном состоянии. На это есть куча причин из жизни и если вы готовы потрать на это наше время, я могу в этом убедить)

eGavr commented 9 years ago

Вроде бы как решили выше в переписках, что ничего лишнего быть не должно

sipayRT commented 9 years ago

Я могу хотеть держать эталонные файлы в опрятном состоянии.

имхо, эталоны без комментариев - это и есть опрятность. Эталон - это то, что должно сгенериться в результате работы шаблонизатора. В итоговый результат не нужно записывать что-то еще. А название эталона должно говорить что там тестируется.

eGavr commented 9 years ago

Давайте определяться...

@sipayRT всех убедил, что игнорировать комментарии мы не будем, и тесты должны быть без лишних комментариев? Conditional comments будут сравниваться по общему принципу (то есть не будем учитывать порядок следования атрибутов в тэгах)

sipayRT commented 9 years ago 
<head>
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <!--[if lt IE 9]>
       <script src="//yastatic.net/es5-shims/0.0.1/es5-shims.min.js"></script>
    <![endif]-->
    <meta charset="utf-8">
    <title>bem-components: select</title>
    <!--[if gt IE 8]><!-->
        <link rel="stylesheet" href="_simple.css">
    <!--<![endif]-->
    <!--[if IE 8]>
        <link rel="stylesheet" href="_simple.ie8.css"/>
    <![endif]-->
    <script type="text/javascript" charset="utf-8" src="//yastatic.net/jquery/2.1.3/jquery.min.js"></script>
</head>
persidskiy commented 9 years ago

@eGavr Нет, меня он не убедил.

Очень часто возникает необходимость объедить тесты, которые про одно и то же в один тестовый файл. Например, у меня есть блок dropdown, он работает с двумя блоками button и link, для каждого из случаев у меня написаны разные шаблоны. При этом в этих шаблонах есть разные условия про разные способы передачи параметров и разные модификаторы. В случае dropdown в Лего, там по 4 явно выраженных кейса с каждым свитчером. Я как порядочный разработчик хочу добиться хорошего покрытия и проверить все возможные варианты передачи параметров. Скажите, я могу хотеть сделать по файлу с тестами на каждый свитчер, чтобы я на одном экране мог сразу видеть, какие кейсы я покрыл? При этом я бы очень хотел их аккуратно отформатировать и подписать комментариями. Кроме этого, на этапе разработки тестов очень часто надо редактировать получившийся эталонный html. Для этого можно легко использовать мультивыделение в редакторе. В случае с несколькими файлами нужно их каждый раз открывать и ты начинаешь в них путаться. Тут тонкий момент. Когда у тебя есть 2 файла(один с bemjson, другой эталонный) ты в голове можешь держать мысль "мне надо во втором тесте исправить то-то"; переключаешься в другой файл и работаешь с ним. Если бы в моем случае было 8 файлов вместо 2, было бы сложно между ними переключаться. А еще бывает так, что на другом уровне переопределения доопределены шаблоны и для этого уровня нужно готовить отдельный файл эталонов. Таким образом, мы имеем 3 файла против 12. Дальше комбинаторика работает в полную силу. И таких блоков достаточно много.

А еще бывает нужно сгенерировать тест-кейсы динамически по какому-то заданному упрощенному массиву параметров, чтобы точно проверить все аспекты поведения. Как в таком случае быть? Тоже выносить тесты по разным файлам?

Так что если вы действительно хотите сделать ваш инструмент удобным потребителям, то давайте работать в этом направлении. Еще раз хочу напомннить, что html-differ в моем понимании, это инструмент, который позволяет сравнивать значимые части html. И conditional comment здесь значимый элемент. Он отвечает за загрузку разных стилей и влияет на итоговый результат работы html в браузере, а обычные комментарии - не значимые части, они нужны только для удобства разработчиков.

eGavr commented 9 years ago

С точки зрения реализации, мне не принципиально как делать ;) В итоге все будет сводиться к тому, какие опции заюзать. Опцию ignoreComments я расширю и она будет такой:

ignoreComments: {
    normalComments: true || false,
    conditionalComments: true || false
}

а вот какие значения указать этой опции в БЭМ-пресете уже договариваться вам.

persidskiy commented 9 years ago

Так в чем тогда проблема? Я же и просил - дайте возможность писать простые комментарии в эталонных шаблонах. А вы пытаетесь убедить меня, что я не так тесты пишу)

eGavr commented 9 years ago

Я пишу html-differ) А не тесты на шаблоны ;) и спор возник у тебя с человеком, который их тоже пишет и что-то в этом понимает.

arikon commented 9 years ago

@eGavr Ок, Женя, запили эту фичу. Подробнее отвечу в Стартреке.

sipayRT commented 9 years ago

@Coltspb я все равно не понимаю смысла в комментариях. Эталон - это эталон, а не кусок кода, который можно комментировать. Насчет кучи тестов в одном файле - ты пишешь много проверок в одном it, когда пишешь тесты на js? Если да, то как ты потом понимаешь что именно не так, когда этот тест падает? Такая же (ну почти такая же) ситуация и тут - ты хочешь в одном тесте проверить кучу кейсов. Но если Жека расширяет ignoreComments, то все это уже не важно :)

eGavr commented 9 years ago

Важно, чтобы вы решили, какой БЭМ-пресет будет! Расширить - я расширю, НО все это крутится в enb-bem-tmpl-specs, и эта тулза использует html-differ с определенными опциями, и надо определиться всем, как именно конфигурировать проверку комментов. Либо проверяем обычные комменты, либо нет.

blond commented 9 years ago

Важно, чтобы вы решили, какой БЭМ-пресет будет!

Понятно же, что такой:

ignoreComments: {
    normalComments: true,
    conditionalComments: false
}
eGavr commented 9 years ago

@sipayRT не видит смысла игнорировать обычные комментарии, так как они не могут туда попасть, если их не писать дополнительно руками.

blond commented 9 years ago

@sipayRT не видит смысла игнорировать обычные комментарии, так как они не могут туда попасть, если их не писать дополнительно руками.

Инструменты не должны никого ограничивать без реальной на то необходимости. Поэтому пресет надо подбирать таким, чтобы используя его были довольны все, и те кто хочет писать коментарии, и те, кто не хочет писать коментарии.

eGavr commented 9 years ago

@andrewblond , может сделать так, чтобы conditional comment проверялись всегда, а конфигурировать только проверку обычный комментов?

А смысл НЕ проверять conditional comments? они ломают работоспособность, если отключить проверку и т д.

sipayRT commented 9 years ago

@sipayRT не видит смысла игнорировать обычные комментарии, так как они не могут туда попасть, если их не писать дополнительно руками.

@eGavr да, смысла я не вижу. Но если можно расширить инструмент так, чтобы все остались довольны, то так и делай ;)

blond commented 9 years ago

может сделать так, чтобы conditional comment проверялись всегда, а конфигурировать только проверку обычный комментов?

По мне идея годная, я тоже не вижу смысла не проверять conditional comments.

eGavr commented 9 years ago

Хорошо! Тогда решено, что опция в структуре не меняется и остается как и была, только она не зависимо от ее значения всегда учитывает conditional comments и их содержимое проверяет как обычный HTML.

Для БЭМ-пресета эта опция будет принимать значение true, то есть в итоге обычные комментарии всегда игнорируются (по сильной просьбе @Coltspb )

sipayRT commented 9 years ago

может сделать так, чтобы conditional comment проверялись всегда, а конфигурировать только проверку обычный комментов?

Да, мне тоже ок