-
-
Notifications
You must be signed in to change notification settings - Fork 18
Initial smoke for usage of jest #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a1d2026
853164f
2190651
c81cbe5
44722a2
a94c573
d420ffd
b039963
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
{ | ||
"presets": [ | ||
["@babel/preset-env", { "targets": { "node": "current" } } ], | ||
"@babel/preset-typescript", | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
module.exports = { | ||
verbose: true, | ||
projects: [ | ||
'<rootDir>' | ||
], | ||
testMatch: [ | ||
"**/__tests__/**/*.[jt]s?(x)", | ||
"**/test/**/*.[jt]s?(x)", | ||
"**/?(*.)+(spec|test).[jt]s?(x)" | ||
], | ||
testPathIgnorePatterns: [ | ||
'/node_modules/', | ||
'.d.ts$', | ||
'<rootDir>/test/fixtures', | ||
'<rootDir>/test/helpers' | ||
], | ||
transform: { | ||
'^.+\\.[jt]sx?$': 'babel-jest', | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import { ExecutionOptions } from "../../src/utils/execution_options"; | ||
import { Exercise } from "../../src/exercise"; | ||
import { set as setGlobalLogger, Logger } from "../../src/utils/logger"; | ||
import { BootstrapResult } from "../../dist/utils/bootstrap"; | ||
|
||
export function bootstrap({ exercise, ...overrides }: { exercise: string } & Partial<ExecutionOptions>): Omit<BootstrapResult, 'solution'> { | ||
const options = new ExecutionOptions({ | ||
debug: false, | ||
console: false, | ||
output: '__fake__', | ||
inputDir: '__fake__', | ||
dry: true, | ||
templates: true, | ||
exercise, | ||
...overrides | ||
}) | ||
|
||
const logger = setGlobalLogger(new Logger(options)) | ||
|
||
return { | ||
options, | ||
exercise: new Exercise(exercise), | ||
logger | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import { Solution } from "../../src/solution"; | ||
import { Exercise } from "../../src/exercise"; | ||
|
||
export class InlineSolution extends Solution { | ||
/** | ||
* Create a new solution reference | ||
* | ||
* @param rootDir the path to the root directory of the solution | ||
* @param exercise the exercise this solution belongs to | ||
*/ | ||
constructor(private readonly solutionFiles: string[], exercise: Exercise) { | ||
super('__fake__', exercise) | ||
} | ||
|
||
/** | ||
* Read the solution file(s) | ||
* | ||
* @param n number of files to return | ||
* @returns promise that resolves all the files at once | ||
*/ | ||
public async read(n = 1): Promise<Buffer[]> { | ||
|
||
return Promise.all( | ||
Object.keys(this.solutionFiles) | ||
.slice(0, n) | ||
.map(name => Buffer.from(this.solutionFiles[name], 'utf8')) | ||
) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
import { Runner } from '../src/runner' | ||
import { Analyzers } from '../src/analyzers' | ||
import { TwoFerAnalyzer } from '../src/analyzers/two-fer' | ||
import { InlineSolution } from './helpers/inline-solution' | ||
import { bootstrap } from './helpers/bootstrap' | ||
|
||
const { options, exercise } = bootstrap({ exercise: 'two-fer' }) | ||
|
||
|
||
describe('When running analysis', () => { | ||
it('can approve as optimal', async () => { | ||
|
||
const solutionContent = ` | ||
export const twoFer = (name = 'you') => { | ||
return \`One for \${name}, one for me.\`; | ||
}; | ||
`.trim() | ||
|
||
const solution = new InlineSolution([solutionContent], exercise) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In short: this is possible 👍, using an 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.from(programString)
// => a solution
createSolutionFromString(programString)
// => a solution The right move forward is probably turning
This is not a good idea:
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Not sure about the definition of value object, I would think to some kind of pojo, but the solution has that
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 ( 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes,
Correct ✔️ |
||
const analyzer = new TwoFerAnalyzer(solution) | ||
|
||
const output = await Runner.call(analyzer, options); | ||
expect(output.status).toBe('approve_as_optimal'); | ||
expect(output.comments.length).toBe(0); | ||
}) | ||
|
||
it('can approve with comment', async () => { | ||
|
||
const solutionContent = ` | ||
const twoFer = (name = 'you') => { | ||
return \`One for \${name}, one for me.\`; | ||
}; | ||
|
||
export { twoFer } | ||
`.trim() | ||
|
||
const solution = new InlineSolution([solutionContent], exercise) | ||
const analyzer = new TwoFerAnalyzer(solution) | ||
|
||
const output = await Runner.call(analyzer, options); | ||
expect(output.status).toBe('approve_with_comment'); | ||
expect(output.comments.length).toBeGreaterThanOrEqual(1); | ||
}) | ||
|
||
it('can dissapprove with comment', async () => { | ||
|
||
const solutionContent = ` | ||
export const twoFer = (name) => { | ||
return \`One for \${name || 'you'}, one for me.\`; | ||
}; | ||
`.trim() | ||
|
||
const solution = new InlineSolution([solutionContent], exercise) | ||
const analyzer = new TwoFerAnalyzer(solution) | ||
|
||
const output = await Runner.call(analyzer, options); | ||
expect(output.status).toBe('disapprove_with_comment'); | ||
expect(output.comments.length).toBeGreaterThanOrEqual(1); | ||
}) | ||
|
||
it('can refer to mentor', async () => { | ||
|
||
const solutionContent = ` | ||
const whomst = 'for' | ||
export const twoFer = (name = 'you') => { | ||
return \`One \${whomst} \${name}, one \${whomst} me.\`; | ||
}; | ||
`.trim() | ||
|
||
const solution = new InlineSolution([solutionContent], exercise) | ||
const analyzer = new TwoFerAnalyzer(solution) | ||
|
||
const output = await Runner.call(analyzer, options); | ||
expect(output.status).toBe('refer_to_mentor'); | ||
}) | ||
}) | ||
|
||
describe('When autoloading analyzers', () => { | ||
it('can find an analyzer based on an exercise', () => { | ||
const ActualAnalyzer = Analyzers.find(exercise) | ||
expect(ActualAnalyzer).toBe(TwoFerAnalyzer) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
type Omit<T, K> = Pick<T, Exclude<keyof T, K>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't be just
"test" : "jest"
?I tried it and it seems to compile what needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, I wanted to have the compilation as a test step.
build
is NOT the same transformation asjest
for now. Jest is relying onbabel
to strip the types,build
is actually compiling via thetsc
typescript compiler.