-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Allow for describe
to return a promise
#2235
Comments
I don't think that makes any sense. Describe is just used for grouping, you don't need to use it and most of the time it isn't useful. You can make everything else async which is what is recommended for tests. |
Happy to discuss this further, just closing to manage the queue but convince me/us and send a PR and we'll see what happens :) |
I agree that it's for grouping, but I think as far as optimal developer experience goes it feels very intuitive to add "group-specific logic" inside of the The most obvious example to me is tests that are generated from the directory structure, usually because of the ease of creating fixtures. For example: import fs from 'fs'
describe('transforms', () => {
const dir = resolve(__dirname, 'fixtures')
const transforms = fs.readdirSync(dir)
beforeEach(() => {
synchronousReset()
})
for (const transform of transforms) {
it(transform, () => {
const testDir = resolve(__dirname, 'fixtures', transform)
const fn = require(testDir).default
const input = fs.readFileSync(resolve(testDir, 'input.txt'), 'utf8')
const output = fs.readFileSync(resolve(testDir, 'output.txt'), 'utf8')
assert.equal(fn(input), output)
})
}
}) In the example, you have a series of fixture folders each with their own Imagine now a developer was going to migrate that to It feels very intuitive to do this: ---import fs from 'fs'
+++import fs from 'fs-promise'
---describe('transforms', () => {
+++describe('transforms', async () => {
const dir = resolve(__dirname, 'fixtures')
--- const transforms = fs.readdirSync(dir)
+++ const transforms = await fs.readdir(dir)
--- beforeEach(() => {
+++ beforeEach(async () => {
--- synchronousReset()
+++ await reset()
})
for (const transform of transforms) {
--- it(transform, () => {
+++ it(transform, async () => {
const testDir = resolve(__dirname, 'fixtures', transform)
const fn = require(testDir).default
--- const input = fs.readFileSync(resolve(testDir, 'input.txt'), 'utf8')
+++ const input = await fs.readFile(resolve(testDir, 'input.txt'), 'utf8')
--- const output = fs.readFileSync(resolve(testDir, 'output.txt'), 'utf8')
+++ const output = await fs.readFile(resolve(testDir, 'output.txt'), 'utf8')
assert.equal(fn(input), output)
})
}
}) Resulting in: import fs from 'fs-promise'
describe('transforms', async () => {
const dir = resolve(__dirname, 'fixtures')
const transforms = await fs.readdir(dir)
beforeEach(async () => {
await reset()
})
for (const transform of transforms) {
it(transform, async () => {
const testDir = resolve(__dirname, 'fixtures', transform)
const fn = require(testDir).default
const input = await fs.readFile(resolve(testDir, 'input.txt'), 'utf8')
const output = await fs.readFile(resolve(testDir, 'output.txt'), 'utf8')
assert.equal(fn(input), output)
})
}
}) In that case, the user doesn't need to be aware that describe is "special" because it is run at the very beginning synchronously, and that even though it co-exists with all of the other Jest-related functions, it doesn't act in the same way. That being said, I don't know the internals of Jest. To me it seems like the reason describe is currently synchronous is purely because "that's how it has always been", but there might be some valid architectural trade-off that makes it that way, which is fair if so. If there isn't anything specifically requiring it to be that way though, it seems like it would be more user-friendly not keep that limitation on it, so that it matches the other functions users are used to. If you still disagree, I'd be curious how you'd solve my current use case. My case is slightly more complex that the example in that I have nested |
I guess put differently what I'm asking for architecturally is for the setup phase to be able to be asynchronous, which I can see might be a big ask. I just think it would be less limiting and end up with a more intuitive interface that way. It would eliminate problems like #949 potentially too, which I assume was a similar use case, except in the external files setup realm. |
You can always have side-effectful setup behavior in
We are hoping to one day rewrite Jasmine and remove it from Jest but that is still pretty far out. If you can submit a PR to add async support to "describe" and hack Jasmine to do that, I'd welcome it. |
+1 async consistency between Basically
Jest being the killer testing tool it is for React, tailoring for this pattern for Redux (which 90% of all serious react apps use) makes a lot of sense. 2 cents |
@cpojer can you provide me some quick direction (maybe just a list of relevant file names and functions) to look at to make describe async? Any other pitfalls you're aware of would be much appreciated. |
This is inside of jest-jasmine2, inside of the vendor folder directly inside of Jasmine. As we are hoping to move away from Jasmine, feel free to try to rewrite this and send a PR :) |
I think it does make sense to let for (const [testFolderName, getFactory] of transformToFolderMap) {
describe(testFolderName, async () => {
for (const folderName of await readDirectory(path.join(__dirname, testFolderName))) {
const folder = path.join(path.join(__dirname, testFolderName, folderName));
if (fs.statSync(folder).isDirectory()) {
const inputPath = path.join(folder, 'input.tsx');
const output = await readFile(path.join(folder, 'output.tsx'));
const result = compile(inputPath, getFactory);
it(folderName, () => {
expect(stripEmptyLines(result)).toEqual(stripEmptyLines(output));
});
}
}
});
} |
Why was this closed? |
@flushentitypacket it was mentioned by @cpojer upstream, closed because it's not an actionable thing right now since Jest is built on Jasmine and it would be hard to change, slash not even sure it is the right move. I think that's super valid, GitHub doesn't have a great solution for long-lived discussions, so sometimes maintainers need to close issues to maintain sanity 😄 |
@ianstormtaylor did you ever find a solution to your use case in the end? I also want to generate tests based on a dynamic list of fixtures, which is obtained async, and I can't think of way to do it. It seems like Jest determines that there are 0 tests in my file by the time I have the async fixture list ready. |
I've found a temporary solution for this just yesterday. At the moment I've wrapped my async script in executable and im calling it sync way in my tests: #!/usr/bin/env node
const myAsyncScript = require('../myAsyncScript')
const args = process.argv.slice(2)
myAsyncScript(...args)
.then(data => {
process.stdout.write(JSON.stringify(data))
process.exit()
})
.catch(() => {
process.exit(1)
}) index.test.js import { execFileSync } from 'child_process'
const result = JSON.parse(execFileSync(require.resolve('../scripts/bin/pathToExecutable'), [...args]).toString())
describe('...', () => {
for (const testCase of result) {
test(testCase.title, () => {
// ...
})
}
}) |
This seems like a useful feature, I think the ask is for the TESTS themselves to be defined by an asynchronous process. I have a similar use case where I need to run my Puppeteer tests against a set of different URLs, where the URLs are defined by calling a Google Sheet. |
This would be nice, and totally does make sense. There doesn't seem to be a way to asynchronously define the tests themselves at the moment – but if you're loading fixtures, that's exactly what you need to do. In my case, I want Puppeteer to hit a styleguide page, then define a Jest test for each component in our styleguide that asserts that it visually matches (using |
Another case of the above is wix/Detox#570 - getting a list of test devices to run tests on is async, you want to run your tests across each device: describe('Attached', async () => {
const devices = adb.devices();
for (const { name } of devices) {
describe(name, () => {
beforeAll(async () => { detox.init(...); });
afterAll(async () => { detox.cleanup(); });
// as usual ...
});
}
}); Though of course that issue is about running them in parallel as well, which jest doesn't really have a reasonable way to do. |
As a workaround solution, Is it possible to enable nested |
See #5673 for a PR. We're not sure if we're gonna pull it in, but it's the closest we've gotten :) |
+1 For an async @cpojer Please consider reopening. |
Why I want async describes: I'd like to make a call to an API and run several tests on the output instead of making a call to the API for every single test. Is there a way to do this without an async describe? |
let data;
beforeAll(async () => {
data = await makeDataCall();
});
test('something', () => {
expect(data).toBe(blablabla);
}); |
This would also be helpful for test.each. Unfortunately right now this throws an error: let data;
beforeAll(async () => {
data = await makeDataCall(); // returns test cases
});
describe('something', () => {
test.each(data.cases)(...); // throws "cannot read property of undefined"
}); |
I'm attempting to use Jest to enforce a set of rules on a set of configuration files. Since there could be N files, I'm attempting to enumerate them and create a test for each one individually, like this: const getAllConfigs = require('../get-all-configs');
let configs;
beforeAll(async () => {
configs = await getAllConfigs();
return true;
});
describe('Validate auth config', () => {
configs.forEach(conf => {
test(`${conf} ~ auth config`, () => {
const f = `../configs/${conf}.js`;
const config = require(f);
expect(config.environments.prod.auth).toBeDefined();
expect(Array.isArray(config.environments.prod.auth)).toEqual(true);
//...
});
});
}); At the
I see that my beforeAll callback can return a promise, but this error seems to indicate that Jest is not waiting for that promise to resolve before executing my If I change my If I could nest tests, or if Aside from putting everything into one big test and using |
In my case I was able to work around this by using |
@atuttle the issue you're having is because tests need to be defined synchronously for Jest to be able to collect them, its the same issue we have internally with |
Alternative workaround: define a function testCaseWithAsyncRequirement(requirementPromise, callback) {
return async () => {
let requirement;
let requirementError;
try {
requirement = await requirementPromise;
} catch (error) {
console.error('condition not satisfied, skipping test');
console.log(error);
requirementError = error !== undefined ? error : new Error('undefined value thrown');
}
if (requirementError === undefined) {
await callback(requirement);
}
};
} and then use it in your test case: const assert = require('assert');
const use = require('./TestUtils').testCaseWithAsyncRequirement;
describe('testCaseWithAsyncRequirement', () => {
it('should be OK', use(Promise.resolve('ok'), async data => {
assert.strictEqual('ok', data);
}));
it('should be skipped and not fail', use(Promise.reject(new Error("")), async data => {
assert.fail(`this should never happen: ${data}`)
}));
it('should be skipped and not fail even though error was undefined', use(Promise.reject(undefined), async data => {
assert.fail(`this should never happen: ${data}`)
}));
it('should fail', use(Promise.resolve('invalid'), async data => {
assert.strictEqual('ok', data);
}));
}); Enjoy (: |
@Jezorko I tried the above solution, but it doesn't work with an async |
This issue is such a bummer :-( |
@anodynos can you show me an example code please? |
Is this dead? Ran into another use case for this. Request some async data and setup an it block per item in a json array. :/ |
You can do it in a beforeAll inside the describe |
Unfortunately you cannot if the describe defines the it blocks in a loop. That’s the main issue... 🤷♀️ |
I believe I have the simplest use case to date. If one wants to generate a Node server (or in my case, a custom class that incorporates a Node server) then test with Supertest, you have to get the app as the first "it" in each test suite! I would love to be able to load my app & Supertest once (during the "describe") and pass the app to each test suite. I've tried "beforeAll" but as other have noted, Jest does not wait for the promise to execute before running nested describes. desc("Run All Tests",() => {
let o:any;
beforeAll(async () => {o = await testServer();});
it("init w/o errors",async () => {
await is(o.master);
await is(o.app);},99999); //test passes because of async/await in "it"
baseApiTests(o); //should return nested test suite using fulfilled "o";
}); "TypeError: Cannot destructure property because we need async describe :) |
Why was this closed? Is this feature "something that will never be done" or "an invalid request"? Otherwise, doesn't it make sense to keep it open to invite PRs or further discussion? |
This is clearly an in-demand feature, it has been popping up again and again going on 4 years now. |
Want to add my two cents for why I think this is a worthwhile feature: Here's how timeouts work now: // Not dynamic; works fine:
const TIMEOUT = 1000;
describe('foo', () => {
test('bar', () => {
// ...
}, TIMEOUT);
}); The above does not meet my use case, because I want to be able to set the timeout dynamically. describe('foo', () => {
let timeout;
beforeAll(async () => {
timeout = await getTimeout();
});
test('bar', () => {
// ...
}, timeout); // Timeout is undefined here
}); If we allow for promises in describe blocks, we'd be able to use async/await for fetching our timeouts: describe('foo', async () => {
const timeout = await getTimeout();
test('bar', () => {
// ...
}, timeout); // Timeout is defined 🎉
}); Thanks everyone for your hard work on Jest, and please let me know if there's a better way to configure test timeouts dynamically on a per-test basis! |
Ahh, anybody looking for a workaround to ☝️ the above use case might be interested to learn about In this case, I'm ok fetching the timeout before the entire suite is run, so I just made one of these: // globalSetup.js
import { getMaxTestDuration } from './some/file.js';
export default async function globalSetup() {
process.maxTestDuration = await getMaxTestDuration();
} Then, in my jest config: globalSetup: './globalSetup.js', Now the timeout is available to use in my tests, like: describe('a', () => {
test('b', () => {
// ...
}, process.maxTestDuration);
}); Perhaps you can front-load some async operations globally instead of on a per-describe-block basis? Worked for me! |
Long term solution to this problem ("I need to do something async before declaring my tests") is probably top level await and using native ESM. Former can be tracked in nodejs/node#30370 and the latter in #9430 |
I mean not really, its tied to |
I feel like this would be valuable to break down tests to single expect statements for better organization. For example I want to make a supertest call in describes for POST /items and be able to have a test for statusCode 200, a test for the body, a test for the headers, etc , etc... rather than having to smash them all into one test or calling supertest for each. |
Just adding my $.02 that though in my case it can be worked around, having async (In my case, I'm using a function which is basically the JS equivalent of a Python context manager, more or less like this: describe("myClient", () => {
const testClient = new MyClient(options);
describe("upload", () => { ... });
describe("download", async () => {
await withTempDir(tempDir => {
it("does what it should", async () => {
await testClient.download('somefile', tempDir);
expect(something).toHaveHappened;
expect(somethingElse).not.toHaveHappened;
});
it("errors when appropriate", () => { ... });
it("handles this edge case", () => { ... });
it("handles this other edge case", () => { ... });
// etc
});
});
}); I have to await |
Usefull feature for jest pupeteer. Especially for loading asyn data, that can be used inside descibe.each. I think it is worth to implement |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Do you want to request a feature or report a bug?
Request a feature.
What is the current behavior?
Right now it seems like
async
functions (or promise-returning functions in general) can only be passed toit
and thebefore*/after*
functions for setting up tests.What is the expected behavior?
It would be nice if
describe
could also allow for passing anasync
function for consistency.In my case, I'm running through a whole bunch of fixtures in a directory and automatically generating a bunch of tests. And to do so, I want to use
await fs.readdir(path)
insidedescribe
to get a list of the fixtures, instead of having to fallback to usingfs.readdirSync(path)
.The text was updated successfully, but these errors were encountered: