Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

@lillo42
Copy link

@lillo42 lillo42 commented Nov 26, 2018

fixes bug: https://github.com/dotnet/corefx/issues/2804

Where can I add test? I didn't find any class to add test and how can I test this?

@ahsonkhan
Copy link

Where can I add test? I didn't find any class to add test and how can I test this?

Maybe in here?

OR

public static partial class DateTimeTests

cc @tarekgh

@ahsonkhan ahsonkhan requested a review from tarekgh November 26, 2018 22:19
@danmoseley
Copy link
Member

@lillo42 the code under CoreLib is not live, it is mirrored from the coreclr repo (it is not used for testing either -- just code reuse in some cases). Instead please make a PR with the fix in https://github.com/dotnet/coreclr/blob/master/src/System.Private.CoreLib/shared/System/Globalization/DateTimeFormat.cs. You can reuse this PR for the tests. Please link the PR's.

To run tests you have in CoreFX against fix you have in CoreCLR privately, please see https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/developer-guide.md#testing-with-private-coreclr-bits

@tarekgh
Copy link
Member

tarekgh commented Nov 26, 2018

please add the test on

corefx/src/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoAbbreviatedMonthGenitiveNames.cs

@ahsonkhan
Copy link

the code under CoreLib is not live, it is mirrored from the coreclr repo (it is not used for testing either -- just code reuse in some cases). Instead please make a PR with the fix in

This doc might help provide some context: https://github.com/dotnet/coreclr/blob/master/Documentation/project-docs/adding_new_public_apis.md

From #33581 (comment)

The details are explained in https://github.com/dotnet/coreclr/blob/master/Documentation/project-docs/adding_new_public_apis.md. The doc talks about adding new APIs, but it applies for bug fixes in the existing APIs as well.

@jkotas
Copy link
Member

jkotas commented Nov 27, 2018

The new link to doc: https://github.com/dotnet/coreclr/blob/master/Documentation/project-docs/changing-corelib.md

@tarekgh
Copy link
Member

tarekgh commented Nov 27, 2018

@lillo42 I am not seeing you opened a PR in coreclr repo. are you going to do that? please link it to this one when you open it. and let's if you need any help. thanks for fixing this issue,

@lillo42
Copy link
Author

lillo42 commented Nov 27, 2018

@tarekgh sorry for delay to open PR, I opened, do you need the number of PR?

@tarekgh
Copy link
Member

tarekgh commented Nov 27, 2018

thanks @lillo42 I already looked at the other PR. Just revert the git ignore file there as it is not related to the change we are doing.

@lillo42
Copy link
Author

lillo42 commented Dec 3, 2018

@tarekgh Should I copy changes I've do on CoreCLR to this Repository? Can I use this PR or create another PR ?

@tarekgh
Copy link
Member

tarekgh commented Dec 3, 2018

@lillo42 you just need to wait the coreclr package that has your changes come to corefx. watch the PR's similar to #33783. such PR's is used to bring the new coreclr builds to corefx.

for code mirroring, this will happen automatically.

@ahsonkhan
Copy link

@dotnet-bot test this please

@tarekgh
Copy link
Member

tarekgh commented Dec 6, 2018

test this please

@tarekgh
Copy link
Member

tarekgh commented Dec 8, 2018

test NETFX x86 Release Build

@tarekgh
Copy link
Member

tarekgh commented Dec 9, 2018

@lillo42 the still still failing.

Unhandled Exception of Type Xunit.Sdk.EqualException
Message :
Assert.Equal() Failure
             ↓ (pos 3)
Expected: 19 GenJun 76
Actual:   19 Jun 76
             ↑ (pos 3)
Stack Trace :
   at System.Globalization.Tests.DateTimeFormatInfoAbbreviatedMonthGenitiveNames.AbbreviatedMonthNames_Format() in D:\j\workspace\windows-TGrou---2a8f9c29\src\System.Globalization\tests\DateTimeFormatInfo\DateTimeFormatInfoAbbreviatedMonthGenitiveNames.cs:line 44

@lillo42
Copy link
Author

lillo42 commented Dec 9, 2018

@tarekgh continue fail, but another test, should I merge branch master :

Windows.10.Amd64.ClientRS4.Open-x86-Release
ThreadPoolScheduler_SchedulesOnThreadPool

Unhandled Exception of Type Xunit.Sdk.NotEqualException
Message :

Assert.NotEqual() Failure
Expected: Not 10 Actual: 10

Stack Trace :

--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.IO.Pipelines.Tests.SchedulerFacts.<ThreadPoolScheduler_SchedulesOnThreadPool>d__11.MoveNext() in D:\j\workspace\windows-TGrou---2a8f9c29\src\System.IO.Pipelines\tests\SchedulerFacts.cs:line 542
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) ```

@jkotas
Copy link
Member

jkotas commented Dec 9, 2018

This failure is unrelated to your change. Opened https://github.com/dotnet/corefx/issues/33937 on it.

@jkotas
Copy link
Member

jkotas commented Dec 9, 2018

@dotnet-bot test NETFX x86 Release Build please

@tarekgh tarekgh merged commit 28e68e5 into dotnet:master Dec 9, 2018
@tarekgh
Copy link
Member

tarekgh commented Dec 9, 2018

Thanks @lillo42 for submitting these PR's. and thanks @jkotas for your help.

@lillo42 lillo42 deleted the fixes-abbreviated-genitive branch December 10, 2018 08:06
jlennox pushed a commit to jlennox/corefx that referenced this pull request Dec 16, 2018
* fix problem to Abbreviated genitive month

* Add test

* Reverse commit

* fixes test

* Skipe .NET Framework

* Add spaces

* Change tab to space
@karelz karelz added this to the 3.0 milestone Dec 21, 2018
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
* fix problem to Abbreviated genitive month

* Add test

* Reverse commit

* fixes test

* Skipe .NET Framework

* Add spaces

* Change tab to space


Commit migrated from dotnet/corefx@28e68e5
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants