Make the adapter localizable#23
Conversation
|
@rashidsarwar, |
rashidsarwar
requested review from
a team,
LukaszMendakiewicz,
rpjohnst and
spebl
LukaszMendakiewicz
left a comment
LukaszMendakiewicz
left a comment
There was a problem hiding this comment.
First part of the review.
| } | ||
| public class PreparingTestRunner : ITestRunner | ||
| { | ||
| public const string TestSetup = "Test setup"; |
There was a problem hiding this comment.
Are these still used anywhere? #Closed
There was a problem hiding this comment.
Yes they are being used in TestAdapter.Tests\TestExecutorTestsBase.cs #Closed
There was a problem hiding this comment.
Let's move them to TestExecutorTestsBase then so we won't have unlocalized strings in this binary. #Closed
| </data> | ||
| <data name="ExecutionTime" xml:space="preserve"> | ||
| <value>{0} Execution took {1}</value> | ||
| <comment>{0} represents an unlocalized name for thread (e.g. "T001519"), {1} represents unlocalized string for time it took (e.g. "3/1/2008 7:00:00 AM")</comment> |
There was a problem hiding this comment.
"3/1/2008 7:00:00 AM" is a time stamp, not time taken. The latter would be e.g. 2 hours 4 minutes 48 seconds. What do we print out here? #Closed
There was a problem hiding this comment.
we print something like "0:00:00.0504161".. updated the comment to reflect it #Closed
| } | ||
| _logger.DebugInfo( | ||
| $"'{executable}' {(matches ? "matches" : "does not match")} {regexSource} regex '{regex}'"); | ||
| string strMatch = " " + Resources.Matches + " " + regexSource + " " + String.Format(Resources.Regex, regex); |
There was a problem hiding this comment.
That's weird that executable, regexSource, and regex are not just fill-ins in Resources.Matches/DontMatch.
Also it is wasteful to create two strings and use only one later on. #Closed
| if (zone.SecurityZone != System.Security.SecurityZone.MyComputer) | ||
| { | ||
| logger.LogError("Executable " + executable + " came from another computer and was blocked to help protect this computer."); | ||
| logger.LogError(String.Format(Resources.ExecutableError,executable)); |
There was a problem hiding this comment.
Missing space after comma. #Closed
| @@ -70,12 +70,12 @@ private static void DiscoverTests(string executable, ITestFrameworkReporter repo | |||
| Action<TestCase> reportTestCases = tc => | |||
There was a problem hiding this comment.
Needs to update the legal header at the top of the file to "8/2017".
Apply to all other files with different month there as well. #Closed
| @@ -1,4 +1,5 @@ | |||
| using System.Linq; | |||
| using System; | |||
There was a problem hiding this comment.
Needs Microsoft-modification header.
Apply to all other modified files missing it as well. #Closed
LukaszMendakiewicz
changed the title
LukaszMendakiewicz
left a comment
LukaszMendakiewicz
left a comment
There was a problem hiding this comment.
End of the pass for this iteration.
Also please squash the fix-up commits before merging.
| <comment>{0} represents an unlocalized test name (e.g. "TestSuite1.Test2")</comment> | ||
| </data> | ||
| <data name="AlreadyInAnalyzer" xml:space="preserve"> | ||
| <value>test case already in analyzer:</value> |
There was a problem hiding this comment.
Capitalize the sentence? #Closed
| <comment>{0} represents unlocalized properties of current object</comment> | ||
| </data> | ||
| <data name="BatchFileMissing" xml:space="preserve"> | ||
| <value>{0} {1} batch file: Did not find file: {2}</value> |
There was a problem hiding this comment.
Can we make "batch file" part of the {1} fill-in (same for the string below)? #Closed
There was a problem hiding this comment.
added a separate BatchFile for "batch file" string #Closed
There was a problem hiding this comment.
fixed the comment as well #Closed
| </data> | ||
| <data name="BatchFileMissing" xml:space="preserve"> | ||
| <value>{0} {1} batch file: Did not find file: {2}</value> | ||
| <comment>{0} represents an unlocalized name for thread (e.g. "T001519"), {1} represents a localized name for test mode (e.g. "Test setup" or "Test teardown"), {2} represents unlocalized file path (e.g c:\test\foo.bat)</comment> |
There was a problem hiding this comment.
Please indicate in the comment which resource names are the potential fill-ins for {1}. #Closed
| <comment>{0} represents an unlocalized process name (e.g notepad.exe), {1} represents unlocalized string for time (e.g. "3/1/2008 7:00:00 AM"), {2} represents localized exception message (e.g. "Cannot load file")</comment> | ||
| </data> | ||
| <data name="CrashTest" xml:space="preserve"> | ||
| <value>reason is probably a crash of test {0}</value> |
There was a problem hiding this comment.
What context this will be printed out in that it is not capitalized? #Closed
There was a problem hiding this comment.
I have capitalized it. I could not track where it would get printed. #Closed
| <comment>{0} represents a unlocalized name of executable (e.g. cmd.exe) file</comment> | ||
| </data> | ||
| <data name="ExecuteSteps" xml:space="preserve"> | ||
| <value>{0} In particular: launch command prompt, change into directory '{1}', and execute the following command to make sure your tests can be run in general.</value> |
There was a problem hiding this comment.
Please indicate in the comment that logically this sentence is a continuation for TroubleShootingLink string. #Closed
| </data> | ||
| <data name="DebuggerAttachMessage" xml:space="preserve"> | ||
| <value>Could not attach debugger to process {0}, no error message available</value> | ||
| <comment>{0} represents a number (e.g. "12" or "14")</comment> |
There was a problem hiding this comment.
Comment seems wrong. #Closed
| </data> | ||
| <data name="DebuggerAttachTime" xml:space="preserve"> | ||
| <value>Debugger attached to process {0}, took {1} ms</value> | ||
| <comment>{0} represents a number (e.g. "12" or "14"), {1} represents a number (e.g. "12" or "14")</comment> |
| catch (InvalidOperationException e) when (e.InnerException is XmlSchemaValidationException) | ||
| { | ||
| throw new InvalidRunSettingsException($"Invalid {GoogleTestConstants.SettingsName}", e.InnerException); | ||
| throw new InvalidRunSettingsException($"{Resources.Invalid} {GoogleTestConstants.SettingsName}", e.InnerException); |
There was a problem hiding this comment.
SettingsName should be a fill-in for the string. #Closed
| <comment>{0} represents a number (e.g. "0" or "1")</comment> | ||
| </data> | ||
| <data name="UnknownVisualStudioVersion" xml:space="preserve"> | ||
| <value>Unknown VisualStudio version: {0}</value> |
| } | ||
|
|
||
| CommonFunctions.ReportErrors(_logger, "test discovery", _settings.DebugMode); | ||
| CommonFunctions.ReportErrors(_logger, Resources.TestDiscovery, _settings.DebugMode); |
There was a problem hiding this comment.
Maybe you should pass down enum here rather than a string like you did with the other one. #Closed
There was a problem hiding this comment.
resolved it. passed down enum #Closed
rashidsarwar
force-pushed
the
dev/rsarwar/localization_fixes
branch
from
82a4fe7 to
f5b511c
Compare
| @@ -163,7 +163,7 @@ public void GetTestsFromExecutable_WithoutPathExtension_ProducesWarning() | |||
| IList<TestCase> testCases = discoverer.GetTestsFromExecutable(targetExe); | |||
There was a problem hiding this comment.
Needs Microsoft-modified header at the top of the file. #Closed
| @@ -17,8 +17,9 @@ namespace GoogleTestAdapter.TestAdapter | |||
| { | |||
There was a problem hiding this comment.
Needs Microsoft-modified header on top. #Closed
| <comment>This sentence is continuation of TroubleShooting string. {0} represents an unlocalized name for thread (e.g. "T001519"), {1} represents unlocalized directory path (e.g c:\test\dir), {2} represents a unlocalized new line string (e.g "\n")</comment> | ||
| </data> | ||
| <data name="ExecutionCancelled" xml:space="preserve"> | ||
| <value>{0} Execution has been canceled: {1}</value> |
There was a problem hiding this comment.
"cancelled" (double l), sorry to have missed this before. #Closed
| <comment>{0} represents an unlocalized string for test name (e.g. "TestSuite1.Test2")</comment> | ||
| </data> | ||
| <data name="TestCaseNotRun" xml:space="preserve"> | ||
| <value>{0} Test cases seem to not have been run - are you repeating a test run, but tests have changed in the meantime? Test cases:{1}</value> |
There was a problem hiding this comment.
"Test cases" should not be capitalized. #Closed
| <comment>this is used in ErrorAndWarning string</comment> | ||
| </data> | ||
| <data name="ErrorAndWarning" xml:space="preserve"> | ||
| <value>The following errors and warnings have occured during {0},{1}:{2}</value> |
There was a problem hiding this comment.
No comma between {0} and {1}. #Closed
| </data> | ||
| <data name="ErrorAndWarning" xml:space="preserve"> | ||
| <value>The following errors and warnings have occured during {0},{1}:{2}</value> | ||
| <comment>{0} represents localized phase of test (e.g. "test discovery"), {1} represents unlocalized hint (e.g. "enable debug mode for more information"), {2} represents a unlocalized new line string (e.g "\n")</comment> |
There was a problem hiding this comment.
Clarify that {0} will be either TestDiscovery or TestExecution. #Closed
| <value>test discovery</value> | ||
| </data> | ||
| <data name="TestExecution" xml:space="preserve"> | ||
| <value>Test execution</value> |
There was a problem hiding this comment.
Should not be capitalized when used in ErrorAndWarning. #Closed
| </data> | ||
| <data name="Results" xml:space="preserve"> | ||
| <value>Test {0}: Expected {1} ms, actual {2} ms</value> | ||
| <comment>{0} represents an unlocalized name for thread (e.g. "T001519"), {1} represents a number (e.g. 5), {2} represents a number (e.g. 5)</comment> |
There was a problem hiding this comment.
{0} represents an unlocalized name for thread (e.g. "T001519"), [](start = 13, length = 63)
This is incorrect #Closed
| </data> | ||
| <data name="XmlNodeParse" xml:space="preserve"> | ||
| <value>XmlNode could not be parsed: '{0}'. Exception message: {1}</value> | ||
| <comment>{0} represents an unlocalized name for thread (e.g. "T001519"), {1} represents localized exception message (e.g. "Cannot load file")</comment> |
There was a problem hiding this comment.
{0} represents an unlocalized name for thread (e.g. "T001519") [](start = 13, length = 62)
This is incorrect #Closed
3 participants
adding localization changes