Анализ кода тестов в два захода

Речь идет не о каком-то «хитром» анализе, который выполняется в два этапа, а просто о двух разных по реализации способах. Нет, речь не будет идти о регулярках и попытках «накостылять» с их помощью. Будем строить синтаксическое дерево JS-кода и «гуляя» по этому дереву находить потенциальные проблемы.

Для начала определим, какие проблемы и в каком коде будем искать. Проверять будем код тестов, который написан с использованием связки mocha/sinon/chai. А проверять будем следующее:

  • Пустые titles для describe и it.
  • Использование stub, spy или restore внутри it.
  • Большое количество assert, expect, should в одном it.

Пример структуры файла с тестами mocha:

describe('d1', {
    describe('d11', function () {
        it('i111', function() {});
        it('i112', function() {});
        it('i113', function() {});
    });
    describe('d12', function () {
        it('i121', function() {});
        it('i122', function() {});
    });
    /* ... */
});

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

Первым мне на глаза попался Esprima:

ECMAScript parsing infrastructure for multipurpose analysis

Так как на выходе Esprima получается EStree-дерево (немного тавтология), то надо бы еще какой-то небольшой модуль для его обхода. Поискал на npmjs.org и нашел esprima-walk. Модуль делает как раз то, что требуется:

A very fast esprima AST node walker with no dependencies.

Скрипт, который будет парсить файлы с тестами, должен как-то получить список проверяемых файлов. Тут без вариантов — glob. В качестве параметра в скрипт будет передаваться glob-шаблон. Хорошо. Но я не хочу сам разбираться с консольными параметрами (да, параметр будет не один). Пусть это делает minimist:

This module is the guts of optimist’s argument parser without all the fanciful decoration.

На данный момент вагон зависимостей для нашего скрипта выглядит так:

"dependencies": {
    "esprima": "^2.7.1",
    "esprima-walk": "^0.1.0",
    "glob": "^6.0.1",
    "minimist": "^1.2.0"
}

Начнем:

var fs = require('fs');
var esprima = require('esprima');
var glob = require('glob');
var parseArgs = require('minimist');
var utils = require('utils.js');
 
var args = parseArgs(process.argv.slice(2));
 
var pattern = args.p;
if (!pattern) {
  process.exit(1);
}

Тут мы ожидаем, что параметром -p будет передан glob-шаблон. Если это не так, то «аварийно» завершаем выполнение скрипта. glob-шаблон парсится так:

glob(pattern, null, function (er, files) {
  if (er) {
    console.log(er);
    process.exit(1);
  }
  var overallFiles = files.length;
  console.log(overallFiles + ' file(s) found.');
  console.time('Execution time');
  for (var i = 0; i < overallFiles; i++) {
    var file = files[i];
    console.log('processing [' + (i + 1) + ' of ' + overallFiles + '] ' + file);
    var tree = esprima.parse(fs.readFileSync(file)).body;
    var parsedTree = utils.parseDescribes(tree);
  }
  console.log(output);
  console.timeEnd('Execution time');
});

glob в коллбеке возвращает список путей, соответствующих переданному шаблону. Эти файлы перебираются по очереди через esprima.parse. Результатом является объект с полем body, которое парсится методом utils.parseDescribes. Этот метод представлен ниже:

// file 'utils.js'
var walk = require('esprima-walk');
 
/**
 * @typedef {object|null} checkerResult
 * @property {{type: string, title: string}} name
 */
 
/**
 * Callback executed for each child node. Called with one argument - child node
 *
 * @typedef {Callback} checkerFunction
 * @param {object} node
 * @return {checkerResult}
 */
 
/**
 * @typedef {object} checker
 * @property {checkerFunction} func
 * @property {string} flag name used to mark node as checked with current checker
 */
 
/**
 * List of checkers used to process each `it`-node
 *
 * @type {checker[]}
 */
var checks = [
  {
    flag: 'emptyTitleProcessed',
    func: function (node) {
      var name = _getNodeName(node);
      if (name === '#EMPTY#') {
        return {
          name: {
            type: 'warning',
            title: '(empty `it` title)'
          }
        };
      }
      return null;
    }
  },
  {
    func: function (node) {
      var sinonIssues = checkSinonIssues(node);
      if (sinonIssues.length) {
        return {
          name: {
            type: 'error',
            title: '(`restore/spy/stub` used in the `it`)'
          }
        };
      }
      return null;
    },
    flag: 'sinonProcessed'
  },
  {
    func: function (node) {
      var expects = tooManyExpects(node);
      var maxExpectCount = 3;
      if (expects > maxExpectCount) {
        return {
          name: {
            type: 'warning',
            title: '(too many `expect` in the `it` - ' + expects + '. Only ' + maxExpectCount + ' allowed)'
          }
        };
      }
      return null;
    },
    flag: 'tooManyExpects'
  }
];
 
/**
 * Get nested value from object
 *
 * @param {object} obj
 * @param {string} prop
 * @returns {*}
 */
function get(obj, prop) {
  var subpathes = prop.split('.');
  while (subpathes.length) {
    var subpath = subpathes.shift();
    obj = obj[subpath];
    if (!obj) return obj;
  }
  return obj;
}
 
/**
 * Move throw all `describe`-nodes and parse nested `describe` and `it` nodes
 *
 * @param {object|object[]}describeNode
 * @returns {Object|Object[]}
 */
function parseDescribes(describeNode) {
  var describes = [];
  walk(describeNode, function (node) {
    if (_checkNode(node, 'describe')) {
      describes.push({
        name: _getNodeName(node),
        describes: parseDescribes(node.arguments[1]),
        its: parseIts(node.arguments[1])
      });
    }
  });
  return describes;
}
 
/**
 * Check if some of `spy/stub/restore` is called in the `it`
 *
 * @param {object} itNode
 * @returns {object[]}
 */
function checkSinonIssues(itNode) {
  var its = [];
  walk(itNode, function (node) {
    if (['restore', 'spy', 'stub'].indexOf(get(node, 'property.name')) !== -1 && !get(node, 'sinonProcessed')) {
      node.chaiProcessed = true;
      its.push(node);
    }
  });
  return its;
}
 
/**
 * Get number of `expect` calls in the `it`
 *
 * @param {object} itNode
 * @returns {number}
 */
function tooManyExpects(itNode) {
  var its = [];
  var counter = 0;
  walk(itNode, function (node) {
    if (['expect'].indexOf(get(node, 'object.callee.name')) !== -1 && !get(node, 'tooManyExpects')) {
      node.tooManyExpects = true;
      counter++;
    }
  });
  return counter;
}
 
/**
 * Check if node has needed name and not already processed with all checkers (@see `checks`)
 * Detects `neededName`, `neededName`.skip and `neededName`.only
 *
 * @param {object} node
 * @param {string} neededName 'it|describe'
 * @returns {boolean}
 * @private
 */
function _checkNode(node, neededName) {
  return (get(node, 'callee.object.name') === neededName || get(node, 'callee.name') === neededName) && !_checkNodeIsProcessed(node);
}
 
/**
 * Try detect obj type basing on {}.toString
 *
 * @param {*} obj
 * @returns {string|null}
 * @private
 */
function _getType(obj) {
  var map = {
    '[object Object]': 'object',
    '[object Array]': 'array',
    '[object String]': 'string'
  };
  var t = {}.toString.call(obj);
  return map[t];
}
 
/**
 * Check if node is processed with all checkers (@see checks)
 * Node should has checks.@each.flag set to `true`
 *
 * @param {object} node
 * @returns {boolean}
 * @private
 */
function _checkNodeIsProcessed(node) {
  for(var i = 0; i < checks.length; i++) {
    if (!get(node, checks[i].flag)) {
      return false;
    }
  }
  return true;
}
 
/**
 * Move throw `it`-node and check it with `checks`
 *
 * @param {object} describeNode
 * @returns {object[]}
 */
function parseIts(describeNode) {
  var its = [];
  walk(describeNode, function (node) {
    if (_checkNode(node, 'it')) {
      checks.forEach(function (checker) {
        var result = checker.func(node);
        node[checker.flag] = true;
        if (result) {
          result.name.t = _getNodeName(node);
          its.push(result);
        }
      });
    }
  });
  return its;
}
 
module.exports = {
  parseDescribes: parseDescribes
};

Что тут происходит? Метод parseDescribes проходит по всем «нодам» (см. AST Node). Если нода является вызовом функции describe, то её «нутро» проверяется функцией parseIts. Эта функция выполняет все checks для нод, которые являются вызовами функции it (см. _checkNode). Что являет собой checks? Это набор «правил», по которым проверяются it-ноды. Каждое правило — это объект с полями flag и func. flag нужен для того, чтоб не применять одно и тоже правило для ноды несколько раз. func являет собой проверку, которую описывает правило. func возвращает null, если нода прошла проверку и проблем не выявлено. Если же проблема есть, то возвращается объект со структурой вида:

name: {
  type: 'error', // error|warning
  title: '(`restore/spy/stub` used in the `it`)' // some human-readable message
}

Далее, в parseIts в полученный объект добавляется поле t с title’ом ноды (через метод _getNodeName). Если title является непустой строкой, то он возвращается без изменений. Если он является пустой строкой, то возвращается '#EMPTY#'. В других случаях — '#EXPRESSION#'.

В итоге, на выходе parseDescribes получается структура вида (просто пример):

[
  {
    "name": "Object",
    "describes": [
      {
        "name": "#method1",
        "describes": [],
        "its": []
      },
      {
        "name": "#method2",
        "describes": [],
        "its": [
          {
            "name": {
              "type": "error",
              "title": "(`restore/spy/stub` used in the `it`)",
              "t": "#EXPRESSION#"
            }
          }
        ]
      }
    ],
    "its": []
  }
]

В нем есть «пустышки» describes и its. Они не нужны и их надо убрать. Сделаем это через метод cleanupTree:

/**
 * Remove empty `describes` and `its` properties from nodes and all child-nodes
 * Remove nodes without `describes` and `its` properties
 *
 * @param {object|object[]} tree
 * @returns {object|object[]}
 */
function cleanUpTree(tree) {
  var type = _getType(tree);
  if (type === 'object') {
    if (get(tree, 'describes.length')) {
      tree.describes = cleanUpTree(tree.describes);
    }
    else {
      delete tree.describes;
    }
    if (!get(tree, 'its.length')) {
      delete tree.its;
    }
  }
  if (type === 'array') {
    var newTree = [];
    for (var i = 0; i < tree.length; i++) {
      var newItem = cleanUpTree(tree[i]);
      if (!get(newItem, 'describes.length') && !get(newItem, 'its.length')) {
        continue;
      }
      newTree.push(newItem);
    }
    return newTree;
  }
  return tree;
}

Если предыдущий массив пропустить через эту функцию, то результат будет таким:

[
  {
    "name": "Object",
    "describes": [
      {
        "name": "#method2",
        "its": [
          {
            "name": {
              "type": "error",
              "title": "(`restore/spy/stub` used in the `it`)",
              "t": "#EXPRESSION#"
            }
          }
        ]
      }
    ]
  }
]

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

var input = [
  {name: 'a', describes: [
    {name: 'b', its: [
      {name: {t: 'c1', title: 'c2', type: 'c3'}},
      {name: {t: 'c4', title: 'c5', type: 'c6'}}
    ]}
  ]}
];
var result = jsonToTable(input);
console.log(result); // [['a', 'b', {t: 'c1', title: 'c2', type: 'c3'}], ['a', 'b', {t: 'c4', title: 'c5', type: 'c6'}]]

Результат работы функции jsonToTable отдаленно поход на монговский unwind. А вот и само описание функции:

function jsonToTable(json) {
  var ret = [];
  var paths = traverse(json)
    .paths()
    .map(function (path) {
      return path.join('.');
    });
 
  _removeShortItems(paths)
    .map(function (p) {
      return p.split('.');
    })
    .forEach(function (path) {
      if (path[path.length - 2] !== 'name') {
        return;
      }
      var obj = json;
      var r = [];
      path.forEach(function (subPath) {
        if (obj && obj.name) {
          r.push(obj.name)
        }
        obj = obj[subPath];
      });
      ret.push(r);
    });
  var divider = '#$!@#';
  var newRet = ret
    .map(function (item) {
      return (item).map(function (i) {
        return JSON.stringify(i);
      }).join(divider);
    });
  return _removeShortItems(newRet)
    .map(function(item) {
      return item.split(divider).map(function (i) {
        return JSON.parse(i);
      });
    });
}
 
/**
 * Remove items if there are some bigger items in the array
 * ['a', 'b', 'ac'] => ['ac', 'b']
 *
 * @param {string[]} arr
 * @returns {string[]}
 * @private
 */
function _removeShortItems(arr) {
  var newRet = [];
  arr
    .sort()
    .reverse()
    .forEach(function (item) {
    for (var i = 0; i < newRet.length; i++) {
      if (newRet[i].indexOf(item) === 0) {
        return;
      }
    }
    newRet.push(item);
  });
  return newRet;
}

Я не утверждаю, что тут все очень правильно и очень оптимизировано, но это работает 🙂 Теперь, используя к примеру cli-table, можно получить красивый вывод результатов в консоли:

output example

На картинке выше под таблицей есть «summary» вида: «error(s): 1 warning(s): 9». Её подсчет реализуется функцией вида:

// file `utils.js`
function getTreeSummary(tree, summary) {
  var type = _getType(tree);
  if (type === 'object') {
    if (tree.type) {
      if (!summary[tree.type]) {
        summary[tree.type] = 0;
      }
      summary[tree.type]++;
    }
    else {
      var keys = Object.keys(tree);
      for(var j = 0; j < keys.length; j++) {
        var key = keys[j];
        summary = getTreeSummary(tree[key], summary);
      }
    }
  }
  if (type === 'array') {
    for(var i = 0; i < tree.length; i++) {
      summary = getTreeSummary(tree[i], summary);
    }
  }
  return summary;
}

Пример использования:

var tree = {
  describes: [
    {
      name: 'a',
      its: [
        {name: {t: '', type: 'error'}},
        {name: {t: '', type: 'warning'}}
      ]
    },
    {
      name: 'b',
      its: [
        {name: {t: '', type: 'error'}},
        {name: {t: '', type: 'error'}},
        {name: {t: '', type: 'warning'}}
      ]
    }
  ]
};
var summary = getTreeSummary(tree, {});
console.log(summary); // {error: 3, warning: 2}
 
// or with predefined summary
var summary = {error: 4, warning: 5, debug: 1};
summary = getTreeSummary(tree, summary);
console.log(summary); // {error: 7, warning: 7, debug: 1}

Получается что задача решена. Выводы по первому способу:

  • Он работает (кэп)
  • Работает не так быстро, как бы хотелось. Причина — не лучший способ обхода нод
  • Умеет работать со skip и only
  • В качестве assertions воспринимает только expect
  • Реализация потребовала большого количества подготовительной работы: определение своего механизма работы правил и ряд дополнительных методов для преобразования полученных результатов (см. cleanUpTree, jsonToTable, getTreeSummary), которые являются довольно узкопрофильными
  • Полученный скрипт является автономной единицей и может быть использован по усмотрению пользователя
  • Все описанные для проверки кода правила выполняются всегда

Весь код в собранном виде выложен на github — test-checker. В README есть описание установки и примеры использования. Функционал по сохранению результатов в файл не был описан выше, но доступен для использования (см. флаг -f). По --help есть описание допустимых параметров.

Второй способ — применение готовых инструментов + свои дополнения к ним.

Готовый инструмент с возможностью расширения через дополнения — это ESLint:

The pluggable linting utility for JavaScript and JSX

В основе eslint лежит тоже самое astree, генерируемое с помощью espree:

Espree started out as a fork of Esprima v1.2.2, the last stable published released of Esprima before work on ECMAScript 6 began. Espree is now built on top of Acorn, which has a modular architecture that allows extension of core functionality. The goal of Espree is to produce output that is similar to Esprima with a similar API so that it can be used in place of Esprima.

То есть, в первом способе, поработав с Esprima, мы сделали себе небольшую базу для перехода к ESLint с espree. Удачно получилось. В документации к ESLint есть раздел по созданию расширений — working-with-plugins, в котором подробно описано, что и как надо делать (вплоть до именования npm-модуля). Ознакомившись с документацией переходим к коду. Напишем правило для проверки пустых title в it и describe:

// file `lib/rules/no-empty-title.js`
/**
 * @fileoverview Rule to disallow use empty title in the `describe` and `it`
 * @author onechiporenko
 * @copyright 2015 onechiporenko. All rights reserved.
 */
 
"use strict";
 
var obj = require("../utils/obj.js");
 
//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
 
module.exports = function (context) {
 
  var toCheck = ['it', 'describe'];
 
  return {
    "CallExpression": function (node) {
      if (toCheck.indexOf(obj.get(node, "callee.name")) !== -1) {
        var firstArgType = obj.get(node, "arguments.0.type");
        if (firstArgType === 'Literal') {
          var title = obj.get(node, "arguments.0.value") || '';
          if (!title.trim()) {
            context.report(node, 'Empty title is not allowed for `{{name}}`.', {name: obj.get(node, "callee.name")})
          }
        }
      }
    }
  };
};

Тут и далее — var obj = require("../utils/obj.js"):

// file `lib/utils/obj.js`
module.exports = {
 
  get: function (obj, path) {
    var subpathes = path.split('.');
    while (subpathes.length) {
      var subpath = subpathes.shift();
      obj = obj[subpath];
      if (!obj) {
        return obj;
      }
    }
    return obj;
  }
 
};

Что получается. Для каждой ноды в анализируемом файле (если она является «CallExpression») выполняется функция, описанная выше. В ней проверяется, что вызываемая функция является той, которую надо проверить (см. toCheck). Если это так, то проверяется ее первый аргумент. Если он является строкой, то проверяется, чтоб она была не пустой. Если она пустая, то для данной ноды генерируется ошибка (см. context.report).

Глядя только на одно это правило уже видно ряд преимуществ использования готового инструмента в сравнении с созданием своих «велосипедов» в первом способе:

  • Он работает быстро
  • Не надо делать никакой подготовительной работы — внутри ESLint уже реализованы и обход дерева, и механизм описания правил, и сбор summary
  • ESLint конфигурируем и правилам можно назначать уровень «угрозы» — error|warning и т.д.
  • Правила вообще можно выключить

Посмотрим теперь на правило, которое проверяет, чтоб внутри it не использовались stub, spy, restore:

// file 'lib/rules/disallow-stub-spy-restore-in-it.js'
/**
 * @fileoverview Rule to disallow usage `stub/spy/restore` in the `it`
 * @author onechiporenko
 * @copyright 2015 onechiporenko. All rights reserved.
 */
 
"use strict";
 
var obj = require("../utils/obj.js");
 
//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
 
module.exports = function (context) {
 
  var disallowed = ["stub", "spy", "restore"];
  var insideIt = false;
 
  return {
    "FunctionExpression": function (node) {
      if (obj.get(node, "parent.callee.name") === "it" && obj.get(node, "parent.arguments.1") === node) {
        insideIt = true;
      }
    },
    "FunctionExpression:exit": function (node) {
      if (obj.get(node, "parent.callee.name") === "it" && obj.get(node, "parent.arguments.1") === node) {
        insideIt = false;
      }
    },
    "Identifier": function (node) {
      if (obj.get(node, "parent.parent.type") !== "CallExpression") {
        return;
      }
      if (obj.get(node, "parent.property.name") !== node.name) {
        return;
      }
      if (insideIt && disallowed.indexOf(node.name) !== -1) {
        context.report(node, "`{{name}}` is not allowed to use inside `it`.", {name: node.name});
      }
    }
  };
};

Так как проверки вызова методов должны проверяться только внутри it, необходимо этот вход отлавливать (выход, кстати, тоже). Это реализуется в «FunctionExpression» и «FunctionExpression:exit» через установку значения переменной insideIt. Внутри «Identifier» проверяется, что текущая нода является частью «MemberExpression» (при чем является «property»). Если это так, то проверяется, входил ли имя ноды в список запрещенных. Если да, то в ноде выдается ошибка. Еще обязательно проверяется, что текущая нода находится внутри it.

Как видно, описание двух правил оказалось довольно легким процессом. Осталось реализовать последнее — об ограничении количества assertions.

// file 'lib/rules/asserts-limit.js'
/**
 * @fileoverview Rule to disallow use more than allowed number of assertions
 * @author onechiporenko
 * @copyright 2015 onechiporenko. All rights reserved.
 */
 
"use strict";
 
var obj = require("../utils/obj.js");
 
//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
 
module.exports = function (context) {
 
  var insideIt = false;
  var assertCounter = 0;
  var assertLimit = context.options[0] || 3;
 
  function isShould(node) {
    return node.name === "should" && obj.get(node, "parent.type") === "MemberExpression" && obj.get(node, "parent.property.name") === "should";
  }
 
  function isExpect(node) {
    return node.name === "expect" && obj.get(node, "parent.type") === "CallExpression" && obj.get(node, "parent.callee.name") === "expect";
  }
 
  function isAssert(node) {
    return node.name === "assert" && obj.get(node, "parent.type") === "MemberExpression" && obj.get(node, "parent.object.name") === "assert";
  }
 
  return {
    "FunctionExpression": function (node) {
      if (obj.get(node, "parent.callee.name") === "it" && obj.get(node, "parent.arguments.1") === node) {
        insideIt = true;
      }
    },
    "FunctionExpression:exit": function (node) {
      if (obj.get(node, "parent.callee.name") === "it" && obj.get(node, "parent.arguments.1") === node) {
        if (assertCounter > assertLimit) {
          context.report(node, "Too many assertions ({{num}}). Maximum allowed is {{max}}.", {
            max: assertLimit,
            num: assertCounter
          });
        }
        insideIt = false;
        assertCounter = 0;
      }
    },
    "Identifier": function (node) {
      if (!insideIt) {
        return;
      }
      if (isShould(node)) {
        return assertCounter++;
      }
      if (isExpect(node)) {
        return assertCounter++;
      }
      if (isAssert(node)) {
        return assertCounter++;
      }
    }
  };
};

В отличии от предыдущего правила, в этом ошибка выдается для «FunctionExpression:exit». Ведь только выходя из it, мы имеем точное количество assertions внутри него. Каждый «Identifier» проверяется на то, является ли он потенциальным assertion’ом — как assert, expect или should.

Откуда вообще брались все эти «MemberExpression», «CallExpression» и т.д? Есть такой проект — astexplorer:

A web tool to explore the AST generated by various parsers.

Заходим туда и пишем код, для которого хотим увидеть ASTree. Структура и типы нод будут во вкладке «tree».

Представленный во втором способе код доступен на github — eslint-plugin-mocha-cleanup. Так же его можно использовать установив через npm:

npm i eslint-plugin-mocha-cleanup

Для представленных способов за кадром остался вопросов тестирования. Очевидно, что в первом способе помимо самих правил надо проверять и все вспомогательные методы. Во втором способе тестируются только правила. Для этого в ESLint есть встроенный механизм — RuleTester. В документации есть пример его использования (working-with-plugins#testing).

Вместо небольшого заключения. Работая над обоими способами я узнал для себя немало нового. Сроки решения изначально поставленной задачи позволяли не торопиться и попробовать решить ее самому «в лоб», а потом уже пойти по проторенной дорожке (используя ESLint). Однако, если бы второй способ не был реализован, то первый был бы чисто тратой времени на самообразование.

Оставить комментарий

Top ↑ | Main page | Back