Skip to content

Show a diff when the public api baselines fail #39108

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

Merged
merged 3 commits into from
Jun 17, 2020

Conversation

orta
Copy link
Contributor

@orta orta commented Jun 17, 2020

This uses the jsdiff library which is already in our dependency tree to echo out the public API baseline differences.

Why

These tend to fail more on master, and you don't get to see what's up: https://github.com/microsoft/TypeScript/runs/779388787?check_suite_focus=true#step:4:170

Alternatives

  • We detect for CI via process.env.CI and always do this for all baselines
  • We use the FourSlash.showTextDiff, I did this initially, but it doesn't do a good job on large files as it prints the entire thing, not a focused subset

Looks like:
Screen Shot 2020-06-17 at 7 14 21 AM


If/when this is +1'd - I'll edit the PR to remove the failing test

@orta orta force-pushed the show_diff_public_API branch 2 times, most recently from f0471c3 to 605c268 Compare June 17, 2020 15:25
@DanielRosenwasser
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 17, 2020

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 605c268. You can monitor the build here.

@orta
Copy link
Contributor Author

orta commented Jun 17, 2020

@andrewbranch
Copy link
Member

@orta how hard would it be to un-red the diff? It’s red because it’s part of the error message but at a glance I read that whole thing as deleted lines.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jun 17, 2020

I guess get rid of the test comment and let's merge so that we can iterate on the UX

@orta orta force-pushed the show_diff_public_API branch from 605c268 to 20a5f69 Compare June 17, 2020 19:54
@orta
Copy link
Contributor Author

orta commented Jun 17, 2020

I did both, will merge on green

@orta orta merged commit d7aa5f3 into microsoft:master Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants