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

Enable the commented out test and convert folder entries to sorted list #22450

Merged
merged 2 commits into from
Mar 12, 2018

Conversation

sheetalkamat
Copy link
Member

Trying out the tests since they dont fail on local machine

@sheetalkamat
Copy link
Member Author

@weswigham @mhegazy can you please run this locally to see if you run into errors, I was able to repro the test failure only couple of times before, but not once after this change. Can you please give it try before we can merge this in (since the original PR has passed successfully before i merged it)

@sheetalkamat sheetalkamat changed the title WIP - Enable the commented out test and convert folder entries to sorted list Enable the commented out test and convert folder entries to sorted list Mar 10, 2018
@@ -24,14 +24,14 @@ namespace ts {
* Make `elements` into a `NodeArray<T>`. If `elements` is `undefined`, returns an empty `NodeArray<T>`.
*/
export function createNodeArray<T extends Node>(elements?: ReadonlyArray<T>, hasTrailingComma?: boolean): NodeArray<T> {
if (elements) {
if (!elements || elements === emptyArray) {
elements = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks concerning. Does this happen often? Did you catch the code path where this occurres?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did look for exact code path since they are multiple ones but remember seeing this with code fix related node creations being one of them. it was discovered as part of #22167 when code fix related tests would cause those module resolution tests to fail....

@sheetalkamat
Copy link
Member Author

Merging this after offline discussion with @mhegazy

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.

2 participants