Skip to content
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 it to return <Promise> #970

Closed
wants to merge 1 commit into from
Closed

Conversation

aaronabramov
Copy link
Contributor

to review: jest-utils/jasmine-pit and __integration_tests_/promise_it/**

It does not work with jasmine1 yet. If the direction is ok i'll add jasmine1 support to this PR.

@cpojer
Copy link
Member

cpojer commented May 9, 2016

Can you rebase this? We should try it on www.

};
}
// env.xit will not run anyways so we don't have to worry about it
env.it = makePromiseIt(env, 'it');
Copy link
Member

Choose a reason for hiding this comment

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

let's make this so we it looks like this: env.it = promisifyIt(env.it, env);? Passing function names as strings is dangerous.

Also, can we do:

env.pit = env.it? We shouldn't break pit just yet – we can deprecate it and provide a codemod so people can switch and we'll remove it later in a major version. Here is what I recommend (in case this diff doesn't impact perf too much):

  • Alias it to pit.
  • Codemod everything on Facebook's www to it.
  • Add a warning to pit.

@@ -0,0 +1,66 @@
/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

left untouched for jasmine1

@aaronabramov
Copy link
Contributor Author

aaronabramov commented May 14, 2016

separated 1 and 2 pits. that makes much more sense now :)

@aaronabramov
Copy link
Contributor Author

oh... i forgot i need to revert the runtime specs

@aaronabramov
Copy link
Contributor Author

@cpojer ok. i reverted Runtime/__tests__/* and aliased it to global.pit
jasmine1 also passes, so let's give it a try :)

@cpojer
Copy link
Member

cpojer commented May 17, 2016

@facebook-github-bot import

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to Phabricator to review.

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants