Кросс-тестирование матчеров

Issue #37 closed
Oleg Sychev repo owner created an issue

Originally reported on Google Code with ID 37 ``` Сейчас каждый из 3-х собственных матчеров имеет собственный набор юнит-тестов. При этом тесты на матчинг в целом (match, match_inner) покрывают общую функциональность всех матчеров (в отличие от тестов для специфических функций). Необходимо использовать их для тестирования всех матчеров - таким образом, выполнив чисто техническую работу, каждый получит в 3 раза больше тестов на матчинг.

Необходимые изменения: 1) выделить для таких тестов (на матчинг в целом) специальный класс (каждому - свой, смешивать пока не надо - видно будет чьи тесты); 2) в начальном методе этого класса - смотрим описание на сайте Moodle как он называется - подключить файл с классом типа вопроса и создать объект, сохранив его в поле, получить из него список доступных матчеров (функция available_engines), выкинуть из него первый (чтобы не тестировать preg_match), а для оставшихся подключить файлы с классами матчеров (см. пример в get_matcher где матчер подключается по имени, имя - ключ в массиве, который вернула available_engines), сам список имен классов-матчеров сохранить в свойстве класса 3) при выполнении теста на матчинг руководствоваться следующим планом: 3.1.) для каждого матчера из списка (подготовленного в п 2) 3.1.1.) создать объект матчера 3.1.2.) проверить его на наличие и типы ошибок: ошибки парсинга надо рапортовать как непройденный тест ассертами, в то время как ошибки принятия узлов (неподдерживаемый узел) это нормально - значит матчер не поддерживает такое выражение; 3.1.3.) если ошибок не было вообще, произвести матчинг и проверить результаты. При проверке результатов учитывать возможности матчера, возвращаемые функцией is_supporting - если что непонятно по константам спрашивайте, я объясню, хотя комменты вроде понятные... 4) при ассертах в тестах на матчинг и ошибках не забывать включать в сообщение имя тестируемого матчера

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

Кто-нибудь приготовит пример одного теста в таком виде + функцию подготовки для класса? Я сейчас занят переводом типа вопроса на Moodle 2.1 который очень хотят наши пользователи (и весьма нетривиальным, т.к. вся система вопросов там переработана)... ```

Reported by `oasychev` on 2011-09-06 20:32:30

Comments (69)

  1. Oleg Sychev reporter

    ``` Теме задан новый владелец в соответствии с распределением задач.

    Пожалуйста, побыстрее подготовьте пример - там всем немало переделывать. Если что-то непонятно выше - спрашивайте, хотя внимательно прочитайте сначала и посмотрите на код 0 там все легко...

    Для примера надо выбрать два-три теста, в т.ч. с неполным совпадением чтобы можно было пройтись по всем константам-возможностям матчера, с подмасками и т.д.... ```

    Reported by `oasychev` on 2011-09-09 13:18:32

  2. Oleg Sychev reporter

    ``` Эту работу можно выполнять в том же клоне, что и сам матчер (желательно без слияния автоматов, но можно заблокировать их accept'ом) ```

    Reported by `oasychev` on 2011-09-09 13:19:22

  3. Valeriy Streltsov

    ``` При подключении файла

    require_once($CFG->dirroot . '/question/type/preg/questiontype.php');

    появляется ошибка:

    Fatal error: Class 'question_shortanswer_qtype' not found in Y:\home\moodle\www\question\type\numerical\questiontype.php on line 39

    Я точно подключаю нужный файл? Cтранно, что ошибка не в нашем модуле... Версия moodle 2.0 ```

    Reported by `vostreltsov` on 2011-09-10 12:12:13

  4. Oleg Sychev reporter

    ``` Странно что он лезет в numerical хотя подключается из preg'а shortanswer (см. строку 17 подключаемого вами файла).

    А глобальный массив QTYPES с ключами в виде имени типа вопроса недоступен без дополнительных подключений? Если и недоступен, можно попробовать подключить lib/questionlib.php - в 2.0 это создавало массив (хотя тормозило программу)...

    ```

    Reported by `oasychev` on 2011-09-10 18:07:51

  5. Valeriy Streltsov

    ``` Проблема решена подключением questionlib.php. ```

    Reported by `vostreltsov` on 2011-09-10 19:38:35

  6. Valeriy Streltsov

    ``` Добавил новый класс. Не получилось проверять матчер на наличие ошибок. Делал это путём empty($matcher->errors). В итоге сделал в тестах на подмаски проверку на их поддержку. Сделал коммит, чтобы узнать - правильно ли я организовал это? ```

    Reported by `vostreltsov` on 2011-09-11 16:42:02

  7. Oleg Sychev reporter

    ``` Гм. Вы читали класс абстрактного матчера? $matcher->errors они того ... protected. Лишний доступ к свойствам класса не есть очень хорошее дело - мало ли кто чего туда напишет... Там есть вполне себе public функции get_errors() и is_error_exists().

    Хуже с типом ошибок - парсинг или непринятие, т.к. в массиве ошибка - это строка, готовая для выдачи пользователю. Если вы читали build_tree, то должны были видеть процесс генерации ошибок непринятия. Их отличительный признак - наличие в конце get_string('unsupported','qtype_preg',$a)... Т.е. по идее надо бы по тексту из unsupported ее отличать, но там внутри и $a с названием узла... А закладываться на конкретный текст бессмысленно - языков может быть много. Тут стоит подумать. По идее, можно подставить в поля $a малопонятные последовательности символов чтобы найти неизменные части. Или, еще лучше, изменить программу так, чтобы в массиве ошибок хранились сами данные об ошибках, а строки генерировались при запросе в get_errors()... Это, видимо, предпочтительнее - но тогда разберитесь , как они обрабатываются из всех источников - сканера, парсера и собственно матчера (непринятие). Это удобно сделать изучив build_tree... (обратите внимание на вызовы highlight_regex для подсветки ошибочного участка в выражениях).

    С поддержкой подмасок дело хитрое. Любой матчер должен принимать скобки - хотя бы потому что большинство малограмотных авторов регулярных выражений не знают других способов управления приоритетом и объяснять им про "неподмасочные" скобки - накладно. На это реально нарвались на первом же внешнем тестере :) Поэтому на ошибки по части скобок рассчитывать не приходится. Для этого есть в матчере is_supporting и константа SUBPATTERN_CAPTURING. Если отвечает да - значит первый и последний индекс должны содержать в 0 индексе полное совпадение, 1-м - первую подмаску и т.д., если нет - то только 0 индекс с полным совпадением. В тестах на подмаски если матчер не поддерживает их захват, я думаю не повредит все же запустить тест и проверить полное совпадение, а вот совпадение на подмаски (индексы 1 и далее) проверять только если поддерживает.

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

    ```

    Reported by `oasychev` on 2011-09-11 19:23:03

  8. Oleg Sychev reporter

    ``` Я могу попытаться изменить работу системы ошибок чтобы сохранялись объекты, но едва ли раньше вторника - в понедельник очень много занятий... ```

    Reported by `oasychev` on 2011-09-11 19:28:01

  9. Valeriy Streltsov

    ``` А что, если ввести константы типа ошибки, и хранить объекты с двумя полями: такой константой и соответствующей строкой? Насколько я понимаю, нужно будет написать этот простейший класс и заменить в preg_matcher строки на такие объекты? ```

    Reported by `vostreltsov` on 2011-09-11 19:58:11

  10. Oleg Sychev reporter

    ``` Раз уж хранить объекты, так лучше уж хранить и строку и все состояние в объектах - мало ли что еще понадобится в будущий раз, опять переделывать? Не вижу смысла переделывать на объекты и хранить только константу. Можно сделать отдельный класс ошибки на каждый вариант, со своим набором данных, унаследованные от общего предка... ```

    Reported by `oasychev` on 2011-09-11 21:10:06

  11. Valeriy Streltsov

    ```

    Я могу попытаться изменить работу системы ошибок чтобы сохранялись объекты, но едва

    ли раньше вторника

    Я возьму на себя, если уж будут какие-то трудности, тогда попрошу помощи... ```

    Reported by `vostreltsov` on 2011-09-13 14:43:49

  12. Oleg Sychev reporter

    ``` Вся работа по преобразованию данных об ошибках в сообщения идет в preg_matcher::build_tree. Нужно перенести ее в конструкторы объектов-ошибок и переписать preg_matcher::get_errors (которая таки должна возвращать строки, дабы не разладились остальные части вопроса), а в массиве ошибок хранить их объекты. Типа class preg_error { public $errormsg; public $index_first; public $index_last; ... (какая еще общая информация найдется для всех ошибок)}; class preg_parsing_error extends preg_error {дополнительная информация из ошибок лексера/парсинга + конструктор, создающий строку из всей информации}; class preg_accepting_error extends preg_error {аналогично для ошибок при принятии};

    Тогда вы сможете использовать is_a() для определения типа ошибки.

    Посмотрите также на функцию preg_matcher::get_match_results - сейчас вытолкну коммит с ней - она возвращает все результаты матчинга в ассоциативном массиве. Вы можете сделать аналогичный массив из ожидаемых результатов для матчинга (это не(!) значит, что его можно будет тупо сравнить с возвращенным get_match_results), с некоторыми поправками (например вместо одного следующего символа у вас будет их множество). Это открывает возможность к тому, чтобы вынести код организации проверки (циклы, ассерты) в специальную функцию, которой будет передаваться выражение, строки и ожидаемые результаты - чтобы не приходилось его дублировать при каждом тесте... Подумайте в эту сторону... ```

    Reported by `oasychev` on 2011-09-13 17:29:56

  13. Valeriy Streltsov

    ``` В функции from_preg_node поле error_flags заполняется только внутри условия, что в движке описан соответствующий класс, иначе возвращается объект preg_node... Разве не нужно заполнять поле error_flags в случае, если нужный узел не описан в движке? А то получается, что принимается операция, которая в движке вообще не описана... ```

    Reported by `vostreltsov` on 2011-09-14 12:52:57

  14. Oleg Sychev reporter

    ``` Существует масса случаев, когда движку не нужен собственный класс для узла. Те же листья... Вы хотите заставить прописывать в движке пустые классы в этом случае? ```

    Reported by `oasychev` on 2011-09-14 13:11:49

  15. Valeriy Streltsov

    ``` Но ведь для этого была введена функция get_engine_node_name. Что мешает ей возвращать preg_leaf'ы? Тогда не будет такой проблемы. Иначе мне, по крайней мере, придётся писать пустые классы. ```

    Reported by `vostreltsov` on 2011-09-14 13:24:19

  16. Oleg Sychev reporter

    ``` Кстати, accept'а в стандартных узлах нету, так что из этого могут выйти ошибки...

    Надо бы проанализировать 3 существующих движка и посмотреть, какие классы они не дублируют. Попытаться вывести формулу, например "для листьев accept по умолчанию true, для операций - false".

    Можно также сделать функцию в движке, которая бы возвращала массив из имен классов, поддерживаемых из preg_node... ```

    Reported by `oasychev` on 2011-09-14 13:36:01

  17. Oleg Sychev reporter

    ``` get_engine_node_name генерирует имя класса если он есть. preg_leaf не рассчитаны на то, что ему повторно вызовут конструктор и передадут самого себя - это глупо... Надо только сделать уже в матчере функцию, которая будет делать accept для потомков preg_node_... если для них нет классов в движке. И в preg_matcher злобно возвращать false чтобы заставить движок явно проверить класс...

    А зачем вам пустые классы создавать при текущей системе?! ```

    Reported by `oasychev` on 2011-09-14 13:42:28

  18. Valeriy Streltsov

    ``` Что-то я погорячился на счёт пустых классов :) Напишу функцию в матчере, делающую accept для preg_node_... ```

    Reported by `vostreltsov` on 2011-09-14 14:06:10

  19. Oleg Sychev reporter

    ``` Я просил вызывать матчер (is_node_acceptable) только для тех классов узлов, для которых в узле нет собственных классов. А вы сделали для всех подряд... Смысл?

    И я что-то не понял, листы-обратные ссылки nfa разве не поддерживает? Их в списке почему-то нет. И в is_supporting зачем-то вы две возможности убрали - это почему?

    По compare_expected_with_obtained . Два раза одно условие повторять - странно - можно и в одно условие два ассерта написать. Я бы не сравнивал след. символ с нулем, к нему '0' например приводится, логичнее пустую строку через === (тем более именно так инициализируется он в конструкторе preg_matcher, могли бы видеть). А вот насчет оставшихся символов надо давать перечень вариантов. Не все матчеры могут легко найти кратчайший путь - например у backtracking с этим проблемы... Они могут взять первый попавшийся, а не искать самый короткий... Я думаю надо давать все альтернативы (кроме лишних повторений в циклах). ```

    Reported by `oasychev` on 2011-09-16 12:44:08

  20. Valeriy Streltsov

    ``` Первое и последнее исправил, добавил поддержку characters_left.

    В is_supporting был убрано потому что пока ещё не реализовано, аналогично для обратных ссылок. Кстати, реализация обратных ссылок лежит на мне? ```

    Reported by `vostreltsov` on 2011-09-16 19:21:06

  21. Oleg Sychev reporter

    ``` По переделке обработки ошибок: 1) я бы вынес классы ошибок в отдельный файл типа preg_errors.php 2) наименования классов я вам предлагал более вразумительные по английски: preg_parsing_error, preg_accepting_error, preg_modifier_error и т.д. Так по правилам языка читается лучше. ```

    Reported by `oasychev` on 2011-09-16 19:25:09

  22. Oleg Sychev reporter

    ``` Логичнее было бы убрать принятие обратных ссылок, чем следующий символ в любой ситуации. Символ ценнее.

    Реализация обратных ссылок - вы имеете ввиду сам лист? match_inner там элементарная, надо тупо запросить строку из матчера. character в общем то тоже, только встает вопрос о том, в каком конкретно месте остановился матчинг. Мне кажется, что отсутствие параметров в character - ошибка, т.к. с ассертами это нереально. Ей, как и match_inner, надо знать строку и позицию начала матчинга узла (не путать с позицией остановки матчинга по несовпадению, для обратных ссылок это разные вещи - боюсь по позиции остановки обратной ссылке будет отработать гораздо труднее, если вообще возможно). Можно пытаться определить и сохранить след. символ сразу при матчинге, но там встает вопрос как его сбрасывать при повторном матчинге через тот же матчер - не делать же для этого рекурсию... Подумайте как лучше будет, только сначала напишите здесь идею, а не сразу в коммит.

    Вызов is_node_acceptable вы поправили, а в самих функциях остался длинный список. Урежьте его, это же не сложно... ```

    Reported by `oasychev` on 2011-09-16 19:35:25

  23. Oleg Sychev reporter

    ``` Поскольку здесь присутствуют все: в течении следующих двух недель, пока меня не будет, дополнительно необходимо сделать следующие вещи всем. 1) Написать по 2 страницы про свой матчер: какие возможности дает метод, в чем его достоинства и недостатки по сравнению с другими, какие основные проблемы возникали и как они были решены. Будем делать общую статью. 2) Научиться читать литературу. Рекомендую вашему вниманию т. наз. "библиотеку колхоза" (легко находится по этому названию) - там огромное количество литературы по Computer Science, есть современные книги по автоматам, можно и по регулярке поискать. А также сайт издательства Springer - www.springerlink.com - но на него надо заходить из политеха (можно попросить на кафедре из 906 или 903, лучше 906 - она для работы студентов предназначена. У политеха - бесплатный доступ до декабря. Литературу читаем с двумя целями: 2.1.) изучить глубже алгоритмы на автоматах и т.д., возможно есть готовые решения на некоторые проблемы или частичные 2.2.) найти, кто решал похожие проблемы - предсказание следующего символа (не обязательно в вопросах) по частичному совпадению в регулярке, пересечение автоматов не с начала и т.д. Нужно понять, что из того, что мы делаем - новое, а что уже делалось. О результатах поиска по литературе через 2 недели написать отчет по сути на 1-2 страницы: читал то-то, нашел такую-то интересную информацию... ```

    Reported by `oasychev` on 2011-09-22 20:55:24

  24. Valeriy Streltsov

    ``` Готово, проверяйте. DFA, кстати, фейлит тесты на простые ассерты и не выходит из цикла нулевой длины... ```

    Reported by `vostreltsov` on 2011-09-29 12:05:25

  25. Oleg Sychev reporter

    ``` Цикл нулевой длины задан регексом? (Потому как автоматом он может его и не создавать попросту).

    Создайте issue о проблемах в DFA-матчере и опубликуйте там регексы и зафейленные тесты (что конкретно не прошло и как). Подпишите меня и Xapuyc7. ```

    Reported by `oasychev` on 2011-09-30 05:42:20

  26. Oleg Sychev reporter

    ``` Я, к сожалению, не могу сейчас запустить код для проверки - тут не на чем...

    Есть мысль - может быть реализовать это тестирование в виде data-driven testing? Посмотрите мою лекцию. Т.е. свести работу составителя тестов не к вызову вашей функции, а к возвращению массива с нужными данными. Так будет короче... ```

    Reported by `oasychev` on 2011-09-30 07:05:05

  27. Valeriy Streltsov

    ``` На счёт data-driven я давно думал, но пока так работает - пускай... У меня висит мёржинг простых ассертов в листья, и много чего ещё. Пока буду делать то, что не сделано вообще. ```

    Reported by `vostreltsov` on 2011-09-30 10:42:12

  28. Oleg Sychev reporter

    ``` Я бы не откладывал, потому что а) это самая важная часть при релизе (остальное можно прикрыть, но базовая работоспособность матчеров важнее всего) б) это надо будет сделать всем и там большой объем методической работы. в) это несложно

    Я бы попросил ко вторнику-среде здесь опубликовать пример, как вы видите, а к пятнице сделать. Чтобы я мог в субботу-воскресенье проверить, потом будет труднее.. ```

    Reported by `oasychev` on 2011-10-03 05:28:43

  29. Valeriy Streltsov

    ``` Данные представляются массивом (для примера пишу рандомные значения): array( 'regex'=>'^[-.\w]+[a-z]{2,6}$', 'str'=>'ff', 'is_match'=>true, 'full'=>true, 'index_first'=>array(0=>0), здесь ключами являются номера подмасок 'index_last'=>array(0=>2), 'left'=>array(0), разные матчеры могут дать разную длину до совпадения 'next'=>'');

    Тестовая функция: function some_kind_of_test() { $this->test(array(...)); тут таким же образом задать нужный массив }

    Функцией test() будет нынешняя функция compare_expected_with_obtained(), очевидным образом переработанная. ```

    Reported by `vostreltsov` on 2011-10-03 11:01:40

  30. Oleg Sychev reporter

    ``` Для data-driven лучше бы наоборот, чтобы ваша функция вызывала их, а у них возвращался массив. И учтите что на один регекс может быть несколько тестов с разными строками и результатами. ```

    Reported by `oasychev` on 2011-10-03 11:16:04

  31. Valeriy Streltsov

    ``` Тогда так: функция test() через get_class_methods вызывает функции с тестами. Тестовые функции возвращают такие массивы: array( 'regex'=>'...', arr1, arr2, arr3...); где arr-i - такие же массивы как в сообщении выше, только без поля regex.

    ```

    Reported by `vostreltsov` on 2011-10-03 11:30:06

  32. Oleg Sychev reporter

    ``` Лучше таким методам при этом задать определенную схему наименования (например имена должны начинаться с...), на случай если понадобятся другие методы в классе. И тогда фильтровать по этому признаку.... ```

    Reported by `oasychev` on 2011-10-03 11:32:22

  33. Valeriy Streltsov

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

    Reported by `vostreltsov` on 2011-10-03 11:34:56

  34. Oleg Sychev reporter

    ``` Старайтесь избегать в функциях, возвращающих массивы, длинных строк.

    Они нечитаемые и их тяжело проверять. Удобнее отформатировать по одному параметру на строке, как в комментарии.

    И я бы сделал примерно такую структуру array( 'regex' = > .... , 'tests' => array( массив, где через запятую перечислены тесты из строк и всех остальных данных) ) чтобы стимулировать создание более одного теста на регекс, вообще проверять работу на регексе одной строкой - нонсенс. ```

    Reported by `oasychev` on 2011-10-04 07:43:50

  35. Oleg Sychev reporter

    ``` Всем: Я жду те части статьи, что вы должны были написать за прошедшие две недели. Можно передавать их по расписанию (в среду или пятницу).

    ```

    Reported by `oasychev` on 2011-10-11 18:07:58

  36. Oleg Sychev reporter

    ``` Стрельцов: замечания в комменте 36 исправлены? ```

    Reported by `oasychev` on 2011-10-11 18:08:20

  37. Valeriy Streltsov

    ``` Да, исправлены. Статью передам как придумаю, что написать про обратный nfa... ```

    Reported by `vostreltsov` on 2011-10-11 21:03:29

  38. Oleg Sychev reporter

    ``` Валерий - я, к сожалению, проапгрейдил себе Moodle до 2.1 и теперь в процессе исправления самого вопроса, который надо серьезно переписать, поэтому у меня проблемы с запуском кода под 2.0, пока я в процессе переписывания самого вопроса...

    Вы сможете в пятницу принести ноут с вашей версией кросс-тестирования? ```

    Reported by `oasychev` on 2011-10-19 20:59:46

  39. Oleg Sychev reporter

    ``` 1) по конструктору - существуют функции, возвращающие массив ключей. Также можно удалить элемент массива (проще пока он ключ - просто через unset). Цикл там избыточен. Читайте внимательно работу с массивами. 2) Я что-то не видел, чтобы вы различали между ошибками принятия и ошибками парсинга выражения... Если есть ошибка принятия, то тест просто должен пропускаться для данного матчера. Молча. Ну не поддерживает он таких возможностей, это нормально. 3) Функция assertTrue может получать сообщение для вывода (в которое можно подставить имя тестируемого матчера) вторым параметром, выводите его прямо там, а не ниже... 4) я бы не начинал функции с данными со слова test, т.к. все функции начинающиеся со слова test выполняются как содержащие тесты, а зачем нам их лишний раз вызывать? Вреда нет, но задержки лишние будут. Назовите типа data_for_test_... 5) Функции, связанные с самой системой тестирования _construct, test, check_for_errors надо вынести в отдельный класс и поместить в отдельном файле. Так, чтобы классы с тестами из каждого матчера могли подключить этот файл и унаследоваться от этого класса. Не копировать же этот код каждому...

    ```

    Reported by `oasychev` on 2011-10-20 19:39:41

  40. Valeriy Streltsov

    ``` Замечания исправил. В матчере закомментировал $error_flags (оно вроде бы нигде кроме preg_matcher.php не используется). Из объектов ошибок и так можно подсвечивать неправильные выражения. ```

    Reported by `vostreltsov` on 2011-10-21 15:45:20

  41. Oleg Sychev reporter

    ``` Пока не будем трогать error_flags. Надо решить, будем мы выводить однотипные ошибки или нет сначала. Потом уже и перепишем....

    Я думаю надо будет сделать ограничение на количество выводимых ошибок. Но не сегодня! ```

    Reported by `oasychev` on 2011-10-21 21:55:44

  42. Oleg Sychev reporter

    ``` ВСЕМ! ВАЖНО!

    В основном репозитории размещен код кросс-тестирования матчера. См. файлы crosstester.php (базовый класс, от которого вам всем наследоваться) и test_cross_from_nfa.php (пример готовых кросс-тестов).

    Приоритетнейшая задача: перевести свои юнит-тесты на матчинг в формат кросс-тестов. Для этой задачи создать отдельный клон от своих проектов, чтобы можно было вытянуть эти изменения без других. Ну и смержить сделанные изменения в свои проекты тоже следует... Обратите внимание на некоторые изменения в принятии узлов - для DFA апгрейд кода произведен, для backtracking - нет. ```

    Reported by `oasychev` on 2011-10-21 21:59:07

  43. Oleg Sychev reporter

    ``` При кросс-тестировании если возникает ошибка превышения размеров автомата, значит не проверять этот матчер на этом выражении... ```

    Reported by `oasychev` on 2011-11-12 16:11:16

  44. Oleg Sychev reporter

    ``` Валерий, поправьте кросс-тестинг с учетом новых видов ошибок, чтобы он правильно на них реагировал... ```

    Reported by `oasychev` on 2011-11-12 20:22:18

  45. Valeriy Streltsov

    ``` Нужно ли править? Ведь тест выполняется только при полном отсутствии ошибок. Сейчас проверка на наличие ошибок ругается только на ошибки парсинга и модификаторов. При ошибках принятия узла и построения автомата тест просто молча пропускается... ```

    Reported by `vostreltsov` on 2011-11-12 20:36:36

  46. Oleg Sychev reporter

    ``` Тогда конкретно это править не надо, но я бы переделал check_for_errors: 1) использовал бы AssertTrue при выводе сообщений об ошибках - обычное echo не проваливает тест... 2) в сообщении об ошибке написал бы, что ошибочно регулярное выражение в тесте - если оно не проходит парсинг, то проблема явно не в матчере. И потом добавил собственно описание ошибки errormsg ... 3) ошибки модификаторов, возможно, выводить не стоит - можно пока закомментированную строку оставить - сейчас поддерживается только u (юникод) и i(нечуствительность к регистру), но вдруг один из матчеров добавит новый модификатор, а другой - не сможет? ```

    Reported by `oasychev` on 2011-11-12 20:43:57

  47. Valeriy Streltsov

    ``` Я добавлю AssertTrue(false) отдельной строчкой, вместе с echo. Если выводить сообщение вторым параметром AssertTrue, там не подсветится сама ошибка в регулярном выражении, и всё будет написано в одну строку - нечитабельно, как мне кажется... ```

    Reported by `vostreltsov` on 2011-11-12 21:01:38

  48. Oleg Sychev reporter

    ``` ВСЕМ: кросс-тестирование из backtracking выявило проблему жадности квантификаторов в матчерах.

    Необходимо проверить, что ваш матчер корректно проводит accept для квантификаторов: если поддерживаются только жадные, то нежадные ($greed == false) должны не приниматься; и наоборот. В тестах, подразумевающих нежадные квантификаторы, должен быть использован корректный синтаксис. По умолчанию квантификаторы жадные...

    Отпишитесь здесь о результатах проверки/сделанных исправлениях. ```

    Reported by `oasychev` on 2011-11-19 11:20:40

  49. Oleg Sychev reporter

    ``` Ситуация с issue 60 показала, что в некоторых тестах необходимо обеспечивать также массив возможных продолжающих символов (и, возможно, вообще массив частичных совпадений?)

    Я бы не стал делать его массивом везде, сделав вместо этого проверку is_array в функции кросс-тестинга, т.к. в большинстве тестов массив все же не требуется. Валерий, поправьте кросс-тестинг.

    Алексей - у вас два варианта - в ближайшие пару дней сделать клон с основным репозиторием + только ваш кросс-тестинг (и якорение, если оттестено, может быть там позже - с юнит-тестами конечно) - или Валерий закоммитит ваш файл с указанием авторства в комментарии в свой репозиторий и сделает необходимые правки. Вам было бы удобнее, если бы в колонке автора коммита значились вы.... ```

    Reported by `oasychev` on 2011-11-21 21:23:30

  50. Oleg Sychev reporter

    ``` Более подробный анализ: 1) не стоит делать массивами все или ничего - может быть несколько возможных длин продолжения, но при этом один вариант совпадения и т.д. 2) предлагаю - рассматривать проверяемые параметры по отдельности 2.1.) is_match и full вероятно не могут быть массивами, если совпадение есть, оно должно быть у любого матчера; если есть полное совпадение - оно должно быть найдено любым матчером 2.2.) index_first и index_last - могут быть массивами массивов (несколько вариантов частичного совпадения) или просто массивами (один вариант частичного совпадения) - но оба сразу, разумеется. 2.3.) next вероятно не имеет смысла делать массивов, т.к. в нем и так перечисляется много возможных следующих символов 2.4.) left может быть числом или массивом (при наличии нескольких возможных длин) - причем отдельно от индексов, т.к. не все матчеры могут находить кратчайшую длину и ситуация с несколькими вариантами длин продолжения может встречаться чаще

    Валерий - если согласны - реализуйте, если есть другие мнения - пишите... ```

    Reported by `oasychev` on 2011-11-22 22:00:01

  51. Oleg Sychev reporter

    ``` Валерий, в кросс-тесты добавьте тесты со всеми видами узлов, которые создаются парсером (т.е. которые есть в preg_node) - включая не поддерживаемые никем, типа сложных ассертов, условных подмасок и т.д. Это нужно чтобы быть уверенным, что они будут отброшены по непринятию (тогда ответ не будет проверяться и все пройдет нормально), а не вызовут какие-нибудь сбои, как в случае выше. Выявленные ошибки исправить... Лучше почитать http://pcre.org/pcre.txt и вставить реальные ответы на них (хотя бы одну совпадающую строку), чтобы не иметь проблем далее...

    Остальным - исправить ошибки, если они обнаружатся этими тестами. ```

    Reported by `oasychev` on 2011-11-22 23:32:09

  52. Valeriy Streltsov

    ``` Все-таки next я бы оставил массивом, потому что если первый движок может дать 'ab', другой 'cd' - ожидаемый результат в тесте будет 'abcd'. Тогда если первый реально возвратит 'c' или 'd' тест будет пройден, хотя матчер работает неверно. Или у Медникова, например, сейчас стоят placeholder'ы - что если nfa возвратит символ из строки 'placeholder', хотя он неверный?:) Сейчас тесты будут проходить, а когда placeholder'ы уберутся - будем долго искать что мы такого сделали, что перестали проходить тесты...

    Аналогично и для остальных полей - не думаю, что будет хорошо, если index_first совпадет с первым вариантом, index_last со вторым, а last, например, будет вообще одним числом для всех вариантов. Пропадает строгость, могут возникнуть ситуации с ошибками в тестах... Тестовый набор для матчера должен быть один, и по одному индексу во всех массивах... Массивы обеспечат лишь разные варианты совпадений, но каждый вариант должен быть жестко задан... ```

    Reported by `vostreltsov` on 2011-11-23 11:30:31

  53. Valeriy Streltsov

    ```

    • left будет одним числом для всех вариантов. Но его непременно следует вернуть на использование массива даже для одного варианта.

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

    Так что текущую версию кросс-тестинга я бы только исправил на использование массивов массивов $left. ```

    Reported by `vostreltsov` on 2011-11-23 13:10:47

  54. Oleg Sychev reporter

    ``` ВСЕМ: кросс-тесты доработаны до формата релиза 2.1. Изменения в основном репозитории. Перевести свои тесты матчера в необходимый формат и вытолкнуть их в клоны (Медникову - сделать отдельный клон главного репозитория и добавить туда соответствующий файл).

    Исправить ошибки, выявленные кросс-тестами (кроме случаев подобных изложенным в issue 60, это на следующий релиз).

    Наличие ваших кросс-тестов и прохождение матчером полного их набора (кроме issue 60) является ОБЯЗАТЕЛЬНЫМ условием включения матчера в релиз Preg 2.1 ```

    Reported by `oasychev` on 2011-11-24 00:43:58

  55. Oleg Sychev reporter

    ``` ВСЕМ: в наборе кросс-тестов появились кросс-тесты от backtracking - спасибо Валерию. (Алексей - стыдно!)

    Перетестируйте свои матчеры и исправьте вскрывшиеся ошибки... ```

    Reported by `oasychev` on 2011-11-24 17:47:57

  56. Oleg Sychev reporter

    ``` Дмитрий - еще одной срочной работой является перевод своих тестов в кросс-формат! Он окончательно утвержден, так что будьте добры сделать это быстро - чтобы можно было протестировать все тестеры с их помощью.

    Эта работа важна для релиза и поэтому будет одним из критериев того, будет ваш матчер включен в релиз или нет!

    В понедельник выходит бета. В ней ДКА пока остается. Окончательное решение о включении матчеров в релиз будет принято к релизу. Если с бетой не будет проблем - релиз возможен в течении недели. Так что не теряйте времени.... ```

    Reported by `oasychev` on 2011-11-26 19:44:31

  57. Valeriy Streltsov

    ``` Предлагаю к 2.2 разделить кросс-тесты по признаку не "от какого матчера", а "на какую ситуацию" - отдельный набор для квантификаторов, отдельный для подмасок и т.д... Очень неудобно искать, добавлять и исправлять... ```

    Reported by `vostreltsov` on 2011-12-01 12:18:58

  58. Oleg Sychev reporter

    ``` Сложный момент. Авторство тоже стоит сохранять. Я бы например не против в будущем обзавестись файлами типа test_cross_from_pcre.php и т.д.

    Вопрос скорее в том, какой признак должен быть первичным для группировки (файл), а какой - вторичным (классы внутри файла). Не стоит забывать, что вообще говоря классифицировать ситуации не так уж легко. Есть масса комплексных ситуаций, которые надо тестировать... А grepWin'ом искать довольно удобно...

    Кстати, к тем 16 тестам на квантификаторы нужно и несложно будет сделать их копию без подмасок. Это для ДКА полезно - ему прямое нахождение одного узла квантификатора в другом может быть хуже, а подмаска между ними - беречь от проблем. Да и себя проверить на этом не лишнее... ```

    Reported by `oasychev` on 2011-12-01 22:16:35

  59. Oleg Sychev reporter

    ``` Валерий, помимо 32 новых тестов (16 с подмасками + 16 без них) желательно оперативно перенести нерешаемые пока проблемы в test_cross_future.php - ну хотя бы чтобы меньше нервировать тех, кому еще править свои матчеры... ;) ```

    Reported by `oasychev` on 2011-12-02 21:21:50

  60. Former user Account Deleted

    ``` Тесты ДКА переведены в кросс тесты. ```

    Reported by `Xapuyc7` on 2011-12-03 18:41:26

  61. Oleg Sychev reporter

    ``` Валерий - жду вашего мнения по фейлам НКА на новых тестах.

    Дмитрий, исключения (предупреждения) с функцией reset (как и прочие) надо устранить. ДКА даже на ваших кросс-тестах их выбрасывает... ```

    Reported by `oasychev` on 2011-12-04 12:45:00

  62. Valeriy Streltsov

    ``` Несогласен по поводу тестов как регекс '^ab' и строка 'OabO'. Мы вроде договаривались: 1) Когда совпадения нет, устанавливать index_first[0] за пределы строки как знак того, что прошли до конца строки и не нашли совпадения. В тесте я вижу пару индексов (0, -1). 2) Если путь непроходимый, устанавливать left = 10000000 (нужно такую константу ввести в абстрактный класс, удобней будет). В этом тесте я вижу left = 1, next = 'a'. Даже если и не учитывать ассерт, тогда уж left = 2 надо было ставить, но уж никак не 1.

    Второй момент: a\wa на строке a{a. Насколько я помню, нужно брать последнее(!) совпадение из цикла по строке. Если нужно брать первое и игнорировать такие же - поправьте меня. ```

    Reported by `vostreltsov` on 2011-12-04 13:23:33

  63. Former user Account Deleted

    ``` Где там непроходимый путь? Нет корректных символов, поэтому генерируем символ с которого совпадение может начаться, это а, Оаб неправильно заменить на аб, путь проходимый, а непроходимый путь это a^b, вот тут не подберешь совпадения, насчет лефт==1, это мой косяк. Насколько я помню, автомат должен выбирать совпадение наибольшей длинны, если есть несколько совпадений наибольшей и равной между собой длинны, то следует выбрать из них, то, которое начинается ЛЕВЕЕ. Если я не напутал с терминами, поищи POSIX НКА. ```

    Reported by `Xapuyc7` on 2011-12-04 15:05:58

  64. Oleg Sychev reporter

    ``` В первом тесте индексы надо исправить (см. НКА как ставим индексы при отсутствии совпадения), left=2 next корректный.

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

    Reported by `oasychev` on 2011-12-04 20:10:44

  65. Oleg Sychev reporter

    ``` Работу по организации кросс-тестинга в целом считаю завершенной. Они позволили узнать нам очень много о матчерах, решить часть проблем и поставить более серьезные. Если нет возражений, закрываю. В дальнейшем - при внесении серьезных изменений - может все-таки потребоваться обновление тестов - но это уже отдельная задача...

    Спасибо всем участникам, в особенности Валерию!

    P.S. Задача пройти все тесты остается... ```

    Reported by `oasychev` on 2011-12-19 14:27:35 - Status changed: `Fixed`

  66. Log in to comment