Skip to content

Conversation

@MaStr11
Copy link
Contributor

@MaStr11 MaStr11 commented Jan 24, 2018

Fix for #23837.
Follow up for #24407 and #24424
This is the third of a series of PRs meant to resolve the unit test failures caused by missing localizations.

This PR includes test related to the editor and workspace:

Roslyn.Services.Editor2.UnitTests

Done. Tested local (de-DE) and CI (en-US).

Roslyn.Services.Editor.CSharp.UnitTests

Done. Tested local (de-DE) and CI (en-US).

Roslyn.Services.Editor.VisualBasic.UnitTests

Done. Tested local (de-DE) and CI (en-US).

Roslyn.Services.UnitTests

Done. Tested local (de-DE) and CI (en-US).

Copy link
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this up. As usual, things we don't verify in our CI get broken -- who would have thought!

@jasonmalinowski
Copy link
Member

jasonmalinowski commented Jan 24, 2018

(and now I see the WIP tag, but hey, so far it looks good. 😄)

@MaStr11
Copy link
Contributor Author

MaStr11 commented Jan 24, 2018

@jasonmalinowski I'm done with this batch of fixes now. I'm just waiting for the CI to finish and will remove the WIP in case all tests pass. I think it is save to start the review right now if you want to (I don't expect the CI tests to fail).

@MaStr11 MaStr11 changed the title Localization test failures 3 [WIP] Localization test failures 3 Jan 24, 2018
@jasonmalinowski
Copy link
Member

@jaredpar @tmeschter @jinujoseph Where are we planning on taking these loc test fixes now? dev15.6.x means we'll be taking an insertion, even though this is test only. Typically test-only hasn't fallen under our escrow bar which is fine, but if this will cause trouble we should just move it.

@MaStr11 Looks good. You can ignore my question, this is entirely a question of internal paperwork. 😄

@jaredpar
Copy link
Member

@jasonmalinowski I think we should just move these to dev15.7.x. It looks probably that we're going to be able to setup a Jenkins run to catch these problems at PR time. I'm guessing that's going to cause a couple of other issues to shake out. More runway in dev15.7.x to deal with that.

Given this branch is in escrow it shouldn't affect contributors that much. Most of our work will be off this branch starting next week.

If there are strong feelings about commiting it here I wouldn't push back.

@tmeschter
Copy link
Contributor

@jaredpar @jasonmalinowski Move it to dev15.7.x.

@jasonmalinowski jasonmalinowski changed the base branch from dev15.6.x to dev15.7.x January 25, 2018 21:30
@sharwell sharwell merged commit c8634c5 into dotnet:dev15.7.x Jan 26, 2018
@MaStr11 MaStr11 deleted the FixLocalizationTestFailures3 branch March 2, 2018 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants