Initial smoke for usage of jest

[SleeplessByte]

@tejasbubane I think this will do as initial smoke?

#9

Also adds .babelrc because that's the recommended way to do ts over jest now (NOT ts-jest), should we also look into compiling ts using babel? We lose 4 core features, of which we currently use none.

Not in this PR:

• actual tests (except for the smoke test)
• coverage
• ci

[depsir]

shouldn't be just "test" : "jest" ?

I tried it and it seems to compile what needed

[SleeplessByte]

For now, I wanted to have the compilation as a test step. build is NOT the same transformation as jest for now. Jest is relying on babel to strip the types, build is actually compiling via the tsc typescript compiler.

[depsir]

I like this way to create doubles to make a plain string work with the Analyzer.

I feel the need though to test the analyzer to a lower level, for example to exercise checkStructure without involving the solution, the exercise, the runner

I would love something like

    const solutionContent = `
    export const somethingElse = (name = 'you') => {
      return \`One for \${name}, one for me.\`;
    };
    `.trim()

    const program = parseToTree(solutionContent);
    const analyzer = new TwoFerAnalyzer();
    analyzer.program = program;
    expect(analyzer.checkStructure()).toBe("DisapproveWithComment")

Probably this should be discussed in #16 since it involves changing the structure of the analyzer, but I feel that a minimal smoke test for a simple function should be available in this pr

[SleeplessByte]

I feel the need though to test the analyzer to a lower level, for example to exercise checkStructure without involving the solution, the exercise, the runner

In short: this is possible 👍, using an InlineSolution or something that conforms to the Solution interface:

const analyzerUnderTest = new Analyzer(somethingThatsASolution)
expect(analyzer.method()).toBe(expectedResult)

// Or run it as a whole
const output = analyzerUnderTest.run()
// => AnalyzerOutput({ status: 'some-status', comments: Comment[] })

InlineSolution is really just a ValueObject and passing around strings is Primitive Obsession. I don't think any test benefits from passing around strings 👎 . You can create another test class or method on InlineSolution or global method:

InlineSolution.from(programString)
// => a solution

createSolutionFromString(programString)
// => a solution

The right move forward is probably turning Solution into an interface and create FileSolution and InlineSolution, completely removing the hierarchy, which I only added because I was being lazy 😅

const program = parseToTree(solutionContent);
const analyzer = new TwoFerAnalyzer();
analyzer.program = program;

This is not a good idea:

• requires exposing program which is an internal state that has nothing to do with the goal of the execution which is coupling implementation and not testing behaviour
• assumes that we must parse in order to analyse (which is again testing implementation and not behaviour); there are plenty of valid ways to analyse code and an AST might not be applicable for all JavaScript or TypeScript analyzers -- and I don't like to couple the test in that manner.
• couples the parseToTree with the test; currently this is called only at one single entry point so we can switch out the implementation; but this could be overcome by re-exporting the parsing entry in a local file which is ok.

That said: Running the analyzer without a runner is possible, and we should test it; I will add the tests when testing the analyzer base class -- but not the smoke test. The smoke test is to detect fire, and it should be an reasonably complete e2e test, hitting all the parts. If this fails, something, somewhere, is probably wrong.

[SleeplessByte]

@depsir have a look at #18

[depsir]

I like #18. I think you took the good points from my comment above and put them into code.. I would like to clarify some points anyway

InlineSolution is really just a ValueObject and passing around strings is Primitive Obsession. I don't think any test benefits from passing around strings

Not sure about the definition of value object, I would think to some kind of pojo, but the solution has that read method. Not sure about primitive obsession too, in javascript/typescript but I understand its value.
In the end, you did what i meant in #18, where the analyzer takes the string (wrapped in an Input object) as Input directly, without bootstrap, options, solution and so on

requires exposing program which is an internal state that has nothing to do with the goal of the execution which is coupling implementation and not testing behaviour

yes, it was a bad way to express that i wanted the analyzer to take an ast as input. I tried to use what was available in the current api, even though private. I'm not sure that having an ast should be an implementation detail but I can live with that.

just to be clear for any other reviewer, if we have a very simple program to validate (export const color = "blue") we could in theory use a regex without involving a parser. I cannot find other examples where the parser is not needed but It makes [some] sense

About testing the behavior and not the implementation, I think that at some point we will need to unit test some steps of the analysis, e.g. checkForSolutionWithoutStringTemplate of two fer, and this will probably need to setup the analyzer with an already parsed example, i.e. we cannot provide the input and then call execute to parse it, just to test checkForSolutionWithoutStringTemplate.
But as you said, this is a smoke test, where we want to exercise the entire behavior on some of the possible paths. That's why I referred #16 as more appropriate to discuss these structural changes

[SleeplessByte]

Not sure about the definition of value object, I would think to some kind of pojo, but the solution has that read method.

Yes, Value Objects are just identified by their value but allow you to attach conversion/casting methods, and use instanceof, implements/extends and so forth, so basically a decorated primitive or set of primitives (or other value objects).

just to be clear for any other reviewer, if we have a very simple program to validate (export const color = "blue") we could in theory use a regex without involving a parser. I cannot find other examples where the parser is not needed but It makes [some] sense

Correct ✔️

[SleeplessByte]

Thank you for the review @depsir ❤️