Skip to content

Fix isProgramUptoDate when changing rootFileNames - #36011

Merged
Sheetal Nandi (sheetalkamat) merged 1 commit into
microsoft:masterfrom
yacinehmito:fix-is-program-up-to-date
Jan 21, 2020
Merged

Fix isProgramUptoDate when changing rootFileNames#36011
Sheetal Nandi (sheetalkamat) merged 1 commit into
microsoft:masterfrom
yacinehmito:fix-is-program-up-to-date

Conversation

@yacinehmito

Copy link
Copy Markdown
Contributor

Fixes#36010

I don't know what tests to write to validate that fix

I'll try to see if the current tests run that codepath. In the meantime the fix is there and maybe it's good enough to be merged as-is.

@sheetalkamat

Copy link
Copy Markdown
Member

please add test similar to https://github.com/microsoft/TypeScript/blob/master/src/testRunner/unittests/reuseProgramStructure.ts#L229 where rootNames change but the number of root names don't change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Requesting changes per Sheetal's comments on test location. Thanks!

@yacinehmito

Yacine Hmito (yacinehmito) commented Jan 9, 2020

Copy link
Copy Markdown
ContributorAuthor

Hello. I don't understand what is expected. The test highlighted doesn't seem to involve isProgramUptoDate. It checks whether the program is reused but the logic doesn't rely on isProgramUpToDate (as a matter of fact it doesn't even call it).

@yacinehmitoYacine Hmito (yacinehmito) changed the title Fix isProgramUpToDate when changing rootFileNamesFix isProgramUptoDate when changing rootFileNamesJan 9, 2020
@yacinehmito

Copy link
Copy Markdown
ContributorAuthor

I found which test to update. It was in the same file. Sorry.

describe("unittests:: Reuse program structure:: isProgramUptoDate should return true when there is no change in compiler options and",()=>{

@sheetalkamat

Copy link
Copy Markdown
Member

Yacine Hmito (@yacinehmito) can you please update the test case so we can take this in. Thanks

@yacinehmito

Copy link
Copy Markdown
ContributorAuthor

Yes. I'm swamped at work so I didn't have time to go back to this. Will do so before Monday. Thanks for the patience!

@yacinehmito

Copy link
Copy Markdown
ContributorAuthor

Sheetal Nandi (@sheetalkamat) I have added the tests. Thank you for your patience.

@sheetalkamat
Sheetal Nandi (sheetalkamat) merged commit 4445e11 into microsoft:masterJan 21, 2020
@yacinehmito
Yacine Hmito (yacinehmito) deleted the fix-is-program-up-to-date branch January 21, 2020 20:05
@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for freeto 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.

isProgramUptoDate wrongfully returns true

4 participants

@yacinehmito@sheetalkamat@RyanCavanaugh@ajafff