Skip to content

Use it instead of describe for tests#16172

Merged
3 commits merged into
masterfrom
it
May 31, 2017
Merged

Use it instead of describe for tests#16172
3 commits merged into
masterfrom
it

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented May 31, 2017

The code inside of a describe runs immediately, while the code in an it doesn't run until the test is started. Using it makes life easier if I want to debug something unrelated to these tests.

@ghost ghost requested a review from rbuckton May 31, 2017 14:38
@sandersn
Copy link
Copy Markdown
Member

JS unit test frameworks seem unnecessarily bad.

@rbuckton
Copy link
Copy Markdown
Contributor

The printsCorrectly function contains a call to it, so this would just hide the tests from Test discovery. The call to describe organizes a test suite.

Copy link
Copy Markdown
Contributor

@rbuckton rbuckton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take a look at the makePrintsCorrectly function before committing this. I don't think this is the correct change.

Comment thread src/harness/unittests/printer.ts Outdated
const printsCorrectly = makePrintsCorrectly("printsFileCorrectly");
const sourceFile = createSourceFile("source.ts", `
// Avoid eagerly creating the sourceFile so that `createSourceFile` doesn't run unless one of these tests is run.
const sourceFile = memoize(() => createSourceFile("source.ts", `
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, we can move this to a before hook

@ghost ghost merged commit 315b72d into master May 31, 2017
@ghost ghost deleted the it branch May 31, 2017 22:31
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants