Make a TestUtil to reduce repeated code in tests about mathsteps

[evykassirer]

Marking this as small because I don't think you have to know much about the code to do this, but it does touch a lot of code so in that sense it's not that small.

[nitin42]

Can I be assigned for this?

[evykassirer]

Yes! Thanks :)

[nitin42]

Work in progress just wanted to confirm that this is how the blueprint would look like for the util functions after reducing the code in tests :

'use strict';
const {canSimplifyPolynomialTerms} = require('../../lib/checks');
const {simplifyPolynomial} = require('../TestUtil');

function testCanCombine(exprStr, canCombine) {
  simplifyPolynomial(canSimplifyPolynomialTerms, exprStr, canCombine);
}

describe('canSimplifyPolynomialTerms multiplication', function() {
  const tests = [
    ['x^2 * x * x', true],
    // false b/c coefficient
    ['x^2 * 3x * x', false],
    ['y * y^3', true],
    ['5 * y^3', false], // just needs flattening
    ['5/7 * x', false], // just needs flattening
    ['5/7 * 9 * x', false],
  ];
  tests.forEach(t => testCanCombine(t[0], t[1]));
});

For testing the substeps ,

function testCollectAndCombineSubsteps(exprString, outputList, outputStr) {
  testCACS(collectAndCombineSearch, exprString, outputList, outputStr);
}

This also eliminates the additional imports in all the test files. Placing all of them in a single util file For eg -

require('assert');
require('math');
require('print');
require('flatten');

Let me know about your thoughts on this and I would create a new branch for all the changes to be merged into the master. Also that I have written the test coverage for other functions that are separated, should I merge them along?

Here is the current coverage summary :

=============================== Coverage summary ===============================
Statements   : 93.75% ( 2458/2622 )
Branches     : 87.34% ( 1069/1224 )
Functions    : 98.94% ( 186/188 )
Lines        : 93.74% ( 2439/2602 )
================================================================================

[evykassirer]

testCanCombine is a little different because it's not comparing nodes/strings, it's checking true/false. I think you should probably leave that test the way it is for now, or come up with a general function that would be used like:

function testCanCombine(exprStr, canCombine) {
  TestUtil.testBooleanFunction(canSimplifyPolynomialTerms, exprStr, canCombine);
}

(not sure if testBooleanFunction is the best name, but it's all I could think of right now)

---

for substeps, I'd expect:

function testCollectAndCombineSubsteps(exprString, outputList, outputStr) {
  TestUtil.testSubsteps(collectAndCombineSearch, exprString, outputList, outputStr);
}

---

I think it'd be best if you put up your code in a pull request and we can talk about it there - will be easier to point out things :)

---

Also that I have written the test coverage for other functions that are separated, should I merge them along?

That's awesome! That's related to an assigned issue #58, so I'd say put it up in a separate pull request from this change to add the TestUtil - make sense?

---

Let me know if you have any questions!

[nitin42]

I'll write a more general method for

I think you should probably leave that test the way it is for now, or come up with a general function that would be used like:

Right now I am clearing my messy code. But soon it will be up for the PR.

That's awesome! That's related to an assigned issue #58, so I'd say put it up in a separate pull request from this change to add the TestUtil - make sense?

Your point is valid.

[evykassirer]

sounds good. looking forward to the PR!

[evykassirer]

done! thanks so much @nitin42