feat(test function name): get function name from config

[ZimGil]

Getting test function name from config

related #38

[vercel]

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

[coveralls]

Coverage increased (+0.3%) to 11.111% when pulling 706dd80 on test-function-name-config into 373a9ae on master.

[thatkookooguy]

Just a few nits.

I suggest you run these changes at least once.
Until we have tests (which you can write if you want), you can run it by adding a call to the function when working on the file locally, and removing it once you know it works.

If I didn't check it myself we might have broken the bot by changing the regex, and even though I should test your code as a reviewer, you should test your code as the submitter even more :-P

[thatkookooguy]

In this file, the only attribute you use from config is tdd1tTestFunctionName.

I think you can save a testFunctionName instead in the getTestsFromPR function's scope and not save a reference to the config itself

[thatkookooguy]

mother of all nits: getPullRequestFiles(); is unrelated to the "config" function.

I would create a function inside the getTestsFromPR scope called getTestFunctionName which gets the config and saved it to the scope, while keeping getPullRequestFiles(); in a separate function in this flow (even though not all the functions in the flow are defined as named functions, but basically the anonymous functions should still be atomic if we can.

[ZimGil]

Do you mean like this:

  return getConfig(context)
    .then((userConfig) => getTestFunctionName(userConfig))
    .then((userTestFunctionName) => {
      testFunctionName = userTestFunctionName;
      return getPullRequestFiles();
    })
    .then((files) => files.map((file) => file.patch))
    .then((patches) => patches.join('\n'))
    .then((prPatch) => getTestNames(prPatch, testFunctionName))
    .then((testNames) => {
      context.log.debug({ data: testNames }, 'test names from pull request');

      return testNames;
    });

function getTestFunctionName(config) {
  return _.get(config, 'tdd1tTestFunctionName', 'it');
}

function getTestName(line, config) {
  const testFunctionName = _.get(config, 'tdd1tTestFunctionName', 'it');
  const getTestNameRegex = new RegExp(`${testFunctionName}\\((.*),`);
  const regexTestResult = getTestNameRegex.exec(line);

  return regexTestResult && regexTestResult[1].trim().slice(1, -1);
}

[thatkookooguy]

I meant like this:

function getTestsFromPR(context) {
  let testFunctionName;

  return getTestFunctionName()
    // .then(...)
    // ...

  function getTestFunctionName() {
    return context.config('config.yml')
      .then((config) => _.get(config, 'tdd1tTestFunctionName', 'it'))
      .then((nameFromConfig) => testFunctionName = nameFromConfig);
  }
}

or like this:

function getTestsFromPR(context) {
  let testFunctionName;

  return getTestFunctionName(context)
    .then((functionName) => testFunctionName = functionName)
    // .then(...)
    // ...
}

function getTestFunctionName(context) {
  return context.config('config.yml')
    .then((config) => _.get(config, 'tdd1tTestFunctionName', 'it'));
}

Basically, the first one is defined inside getTestsFromPR so it has access to the context and can set the scope's variable inside the function, while the second one is defined outside of the function, which means it needs the context passed into it and to output the functionName.

Each of them is legit and not too intrusive. The rest of the flow should stay as-is besides passing the testFunctionName instead of the config along to getTestNames & getTestName.

[thatkookooguy]

🎉 This PR is included in version 1.7.0 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀