Skip to content

[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly - #67666

Merged
tarekgh merged 15 commits into
dotnet:mainfrom
deeprobin:issue-23799
Apr 14, 2022
Merged

[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly#67666
tarekgh merged 15 commits into
dotnet:mainfrom
deeprobin:issue-23799

Conversation

@deeprobin

@deeprobindeeprobin commented Apr 6, 2022

Copy link
Copy Markdown
Contributor

Proposal implementation of #23799 (closes#23799)

Proposal

namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}

Current state of implementation

  • Proposal logic / implementation
  • Ref Assembly
  • Tests

/cc @tarekgh
/cc @ChristopherHaws

@ghostghost added community-contribution Indicates that the PR has been added by a community member new-api-needs-documentation labels Apr 6, 2022
@ghost

ghost commented Apr 6, 2022

Copy link
Copy Markdown

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost

ghost commented Apr 6, 2022

Copy link
Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

deeprobin
deeprobin commented Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
@deeprobin

deeprobin commented Apr 6, 2022

Copy link
Copy Markdown
ContributorAuthor

It should be mentioned that I have taken the documentation comments from the current documentation and adapted them accordingly.

I have extended the current test cases accordingly. I assume that these are not flaky.

@ghost

ghost commented Apr 6, 2022

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

Proposal implementation of #23799 (closes #23799)

Proposal

namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}

Current state of implementation

  • Proposal logic / implementation
  • Ref Assembly
  • Tests

/cc @tarekgh
/cc @ChristopherHaws

Author:deeprobin
Assignees:-
Labels:

area-System.Runtime, new-api-needs-documentation, community-contribution

Milestone:-

@tarekghtarekgh self-assigned this Apr 6, 2022
@tarekgh

tarekgh commented Apr 6, 2022

Copy link
Copy Markdown
Member

@deeprobin I'll try to take a look at this PR later today and will get back to you.

tarekgh
tarekgh reviewed Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/TimeSpan.cs Outdated
tarekgh
tarekgh reviewed Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs Outdated
tarekgh
tarekgh reviewed Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs Outdated
tarekgh
tarekgh reviewed Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh reviewed Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs Outdated
tarekgh
tarekgh reviewed Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs Outdated
@danmoseley

danmoseley commented Apr 7, 2022

Copy link
Copy Markdown
Contributor

@deeprobin you might be interested to check that your tests are giving full code coverage of your code:

runtime/docs/workflow/building/libraries/code-coverage.md

Line 53 in b248950

## Code coverage with System.Private.CoreLib code

@deeprobin

deeprobin commented Apr 7, 2022

Copy link
Copy Markdown
ContributorAuthor

@danmoseley
Thanks. I can take a look at that after the other open questions have been clarified here :).

tarekgh
tarekgh reviewed Apr 7, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh reviewed Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs Outdated
tarekgh
tarekgh reviewed Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs Outdated
tarekgh
tarekgh reviewed Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs Outdated
tarekgh
tarekgh reviewed Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs Outdated
tarekgh
tarekgh reviewed Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeOffsetTests.cs
tarekgh
tarekgh reviewed Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeTests.cs
@tarekgh

tarekgh commented Apr 12, 2022•
edited
Loading

Copy link
Copy Markdown
Member

Could you please add tests for TimeSpan new APIs? Also, when writing these tests please ensure using negative values in addition to positive values.

@tarekgh

tarekgh commented Apr 12, 2022

Copy link
Copy Markdown
Member

@deeprobin I left some comments I hope you can address them soon. After that we should be good to go. great work!

@joperezr

joperezr commented Apr 12, 2022•
edited
Loading

Copy link
Copy Markdown
Member

/azp run runtime (Rerunning CI in order to fetch changes merged in main which have fixed the Regex test)

@azure-pipelines

azure-pipelinesBot commented Apr 12, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@dotnetdotnet deleted a comment from azure-pipelinesBotApr 12, 2022
danmoseley
danmoseley reviewed Apr 13, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs Outdated
@tarekgh

tarekgh commented Apr 13, 2022

Copy link
Copy Markdown
Member

@deeprobin will be able to address the remaining feedback soon?

@deeprobin
deeprobin requested a review from tarekghApril 13, 2022 18:11
@tarekgh

tarekgh commented Apr 13, 2022

Copy link
Copy Markdown
Member

The failing test is unrelated and is tracked by the issue #67878

@tarekgh
tarekgh merged commit b2ed250 into dotnet:mainApr 14, 2022
@ChristopherHaws

ChristopherHaws commented Apr 14, 2022

Copy link
Copy Markdown

🎉 Thanks for implementing this @deeprobin!

@akoeplingerakoeplinger mentioned this pull request Apr 14, 2022
Merged
@deeprobindeeprobin mentioned this pull request Apr 14, 2022
Merged
radical added a commit to radical/runtime that referenced this pull request Apr 14, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
e87bda5
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
@radicalradical mentioned this pull request Apr 14, 2022
Merged
radical added a commit to thaystg/runtime that referenced this pull request Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
79c9fde
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
radical added a commit that referenced this pull request Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main' (#68054)
895715e
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
thaystg added a commit that referenced this pull request Apr 26, 2022
@thaystg@radical
[wasm][debugger] Debug on firefox (#61776)
a970720
* First compiling and working version just proxying messages.
* almost working, already showing the cs files
* Working on firefox.
* Use internal e not public.
* Debugging on firefox working.
* Working after the merge
* Keep the TcpListener open and use a random port.
* Show null value.
* - Show JS Callstack
- Skip properties
- Get array value from evaluateAsyncJS and not use the preview value.
* Fix compilation
* Infrastructure to run debugger tests.
* fix merge
* run test.
* Skipping tests that are not passing on Firefox.
* Skipping tests that are not passing on Firefox.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Failed: 0, Passed: 39, Skipped: 203, Total: 242, Duration: 5 m 6 s - DebuggerTestSuite.dll (net6.0)
* Failed: 0, Passed: 66, Skipped: 195, Total: 261, Duration: 9 m 29 s - DebuggerTestSuite.dll (net6.0)
* Using ConditionalTheory and ConditionalFact implemented by @radical.
* Fixing side effect.
* Implemented conditional breakpoints.
Failed: 0, Passed: 74, Skipped: 189, Total: 263, Duration: 8 m 41 s - DebuggerTestSuite.dll (net6.0)
* Fix special characters and pointers.
Failed: 0, Passed: 116, Skipped: 177, Total: 293
* Fix merge
* Run debugger-tests on firefox using codespace
* Starting firefox correctly not stopping in the breakpoint yet.
* Remove unnecessary change
* Fix pause behavior (now showing correctly, pause on breakpoint, pause while stepping)
Start implementing evaluate expressions, working correctly on VSCode.
* Fix local tests.
* Fix missing )
* Passing 190 tests, evaluate expressions working.
* Remove Task.Delays.
Move some attributes from FirefoxMonoProxy to FirefoxExecutionContext.
* Fix container creation
* Trying to run firefox tests on CI.
* Moving file to the right place.
* Trying to run debugger-tests using firefox on CI.
* fixing path
* Missing url to download firefox on helix.
* On run the tests only on linux.
* Trying to download firefox on helix.
* fix error on helix-wasm.targets.
* trying to fix ci
* trying to install firefox on helix.
* Fixing firefox path
* Fix debugger tests on firefox
* fixing profile path
* Install libdbus-glib-1-2 on docker and on codespace
* Trying to run using firefox on CI
* update docker image
* Adding more messages to see errors on CI
* Trying to make it work on CI
* Real test on CI
* Trying to use the firefox machine only to run firefox tests
Retrying connection to Proxy
Remove extra messages added to help to fix CI
* Fix CI
* Fix CI
* Fix CI.
* Remove unnecessary changes.
* Using machine with sudo installed
* Addressing @lewing comments
* Fix run tests on codespace
Using image with python3
* Use default image to build and new image only to run firefox tests
* Fix unrelated change
* Fix ci
* check python version
* Print python versions
* Using image with PIP installed
* Using image with pip updated
* Remove unrelated changes
Increase time to wait for firefox to be ready
* Trying to fix evaluate tests.
* Fix evaluateoncallframe tests
* Trying to fix evaluation tests.
* trying to fix evaluateoncallframetests
* fiz evaluateoncallframetests
* Trying to kill firefox to avoid errors.
* Trying to fix EvaluateOnCallFrameTests
* Fix CI
* Remove failing test
* Fix misctests
* Fix other build errors.
* Trying to fix CI.
* Fix CI
* Remove unecessary message.
* Update src/tests/BuildWasmApps/Wasm.Debugger.Tests/Wasm.Debugger.Tests.csproj
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Merge error while accept @radical suggestion
* Merge error while accept @radical suggestion
* Update src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
Co-authored-by: Ankit Jain <radical@gmail.com>
* Apply suggestions from code review
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Abort the tcp connection if the proxy throws an exception
* Refactor a bit
* Use more compile time checks for chrome vs firefox
* fix pipeline
* Make debugger job names unique by including the browser
* fix runtime-wasm pipeline
* fix firefox ci job
* split into more files
* cleanup
* Add support for running chrome, and firefox tests in the same job
* fix yml
* fix build
* fix build
* fix windows build
* Don't delete profile folder nor pkill firefox
* Delete and create a new profile folder for each execution
* fix helix command line
* [wasm][debugger] Fix tests broken on 'main'
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
* wip
* big refactor
* chrome runs
* ff runs
* ff runs
* cleanup
* cleanup
* cleanup
* change console verbosity to info, for proxy
* More refactoring
* More refactoring, and fix some issues with connections, and other
cleanup
* some cleanup
* fix file name
* Improve cleanup after tests
* some refactoring, fixing some hangs, faster failures etc
* Fix BrowserCrash test for chrome
* fix up logging
* Improve error handling for the proxy running independently
* fix debugging from vscode
* proxy host: add --log-path for logs
* support canceling for the proxy host too, and distinguish different instances of the proxy
* Fix debugger after refreshing the debugged page.
* Fixing chrome debugging.
* Fix startup to work on chrome and also on firefox.
Co-authored-by: Ankit Jain <radical@gmail.com>
@ghostghost locked as resolved and limited conversation to collaborators May 14, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@tarekghtarekghtarekgh approved these changes
+1 more reviewer
@danmoseleydanmoseleydanmoseley left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@tarekghtarekgh

Labels

area-System.DateTimecommunity-contributionIndicates that the PR has been added by a community membernew-api-needs-documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly

6 participants

@deeprobin@tarekgh@danmoseley@joperezr@ChristopherHaws@jeffhandley
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly by deeprobin · Pull Request #67666 · dotnet/runtime · GitHub
Skip to content
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly - #67666
Merged
tarekgh merged 15 commits into
dotnet:mainfrom 
deeprobin:issue-23799
Apr 14, 2022
Merged
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly#67666
tarekgh merged 15 commits into
dotnet:mainfrom 
deeprobin:issue-23799
Conversation
@deeprobin
@deeprobindeeprobin
commented
Apr 6, 2022

Copy link


Copy Markdown

Contributor
Proposal implementation of #23799 (closes#23799)
Proposal
namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}
Current state of implementation
  •  Proposal logic / implementation
  •  Ref Assembly
  •  Tests
/cc @tarekgh

/cc @ChristopherHaws
@ghostghost
added

community-contribution
Indicates that the PR has been added by a community member
new-api-needs-documentation

labels
Apr 6, 2022
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

Note regarding the new-api-needs-documentation label:
This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.
deeprobin
deeprobin
commented
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
@deeprobin
deeprobin
commented
Apr 6, 2022

Copy link


Copy Markdown

ContributorAuthor
It should be mentioned that I have taken the documentation comments from the current documentation and adapted them accordingly.
I have extended the current test cases accordingly. I assume that these are not flaky.
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-runtime

See info in area-owners.md if you want to be subscribed.
Issue Details
Proposal implementation of #23799 (closes #23799)
Proposal
namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}
Current state of implementation
  •  Proposal logic / implementation
  •  Ref Assembly
  •  Tests
/cc @tarekgh

/cc @ChristopherHaws
Author:deeprobin
Assignees:-
Labels:
area-System.Runtime, new-api-needs-documentation, community-contribution
Milestone:-
@tarekghtarekgh
self-assigned this
Apr 6, 2022
@tarekgh
tarekgh
commented
Apr 6, 2022

Copy link


Copy Markdown

Member
@deeprobin I'll try to take a look at this PR later today and will get back to you.
tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/TimeSpan.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

@danmoseley
danmoseley
commented
Apr 7, 2022

Copy link


Copy Markdown

Contributor
@deeprobin you might be interested to check that your tests are giving full code coverage of your code:
runtime/docs/workflow/building/libraries/code-coverage.md

Line 53
in
b248950
## Code coverage with System.Private.CoreLib code
@deeprobin
deeprobin
commented
Apr 7, 2022

Copy link


Copy Markdown

ContributorAuthor
@danmoseley

Thanks. I can take a look at that after the other open questions have been clarified here :).
tarekgh
tarekgh
reviewed
Apr 7, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeOffsetTests.cs
tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeTests.cs
@tarekgh
tarekgh
commented
Apr 12, 2022•

edited

Loading

Copy link


Copy Markdown

Member
Could you please add tests for TimeSpan new APIs? Also, when writing these tests please ensure using negative values in addition to positive values.
@tarekgh
tarekgh
commented
Apr 12, 2022

Copy link


Copy Markdown

Member
@deeprobin I left some comments I hope you can address them soon. After that we should be good to go. great work!
@joperezr
joperezr
commented
Apr 12, 2022•

edited

Loading

Copy link


Copy Markdown

Member
/azp run runtime (Rerunning CI in order to fetch changes merged in main which have fixed the Regex test)
@azure-pipelines
azure-pipelinesBot
commented
Apr 12, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@dotnetdotnet
deleted a comment from azure-pipelinesBotApr 12, 2022
danmoseley
danmoseley
reviewed
Apr 13, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

@tarekgh
tarekgh
commented
Apr 13, 2022

Copy link


Copy Markdown

Member
@deeprobin will be able to address the remaining feedback soon?
@deeprobin
deeprobin
requested a review
from tarekghApril 13, 2022 18:11
@tarekgh
tarekgh
commented
Apr 13, 2022

Copy link


Copy Markdown

Member
The failing test is unrelated and is tracked by the issue #67878
@tarekgh
tarekgh
merged commit b2ed250
into
dotnet:mainApr 14, 2022
@ChristopherHaws
ChristopherHaws
commented
Apr 14, 2022

Copy link


Copy Markdown

🎉 Thanks for implementing this @deeprobin!
@akoeplingerakoeplinger
mentioned this pull request
Apr 14, 2022
 Merged

@deeprobindeeprobin
mentioned this pull request
Apr 14, 2022
 Merged

radical added a commit
to radical/runtime
that referenced
this pull request
Apr 14, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
e87bda5
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
@radicalradical
mentioned this pull request
Apr 14, 2022
 Merged

radical added a commit
to thaystg/runtime
that referenced
this pull request
Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
79c9fde
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
radical added a commit
that referenced
this pull request
Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main' (#68054)
895715e
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
thaystg added a commit
that referenced
this pull request
Apr 26, 2022
@thaystg@radical
[wasm][debugger] Debug on firefox (#61776)
a970720
* First compiling and working version just proxying messages.
* almost working, already showing the cs files
* Working on firefox.
* Use internal e not public.
* Debugging on firefox working.
* Working after the merge
* Keep the TcpListener open and use a random port.
* Show null value.
* - Show JS Callstack
- Skip properties
- Get array value from evaluateAsyncJS and not use the preview value.
* Fix compilation
* Infrastructure to run debugger tests.
* fix merge
* run test.
* Skipping tests that are not passing on Firefox.
* Skipping tests that are not passing on Firefox.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Failed: 0, Passed: 39, Skipped: 203, Total: 242, Duration: 5 m 6 s - DebuggerTestSuite.dll (net6.0)
* Failed: 0, Passed: 66, Skipped: 195, Total: 261, Duration: 9 m 29 s - DebuggerTestSuite.dll (net6.0)
* Using ConditionalTheory and ConditionalFact implemented by @radical.
* Fixing side effect.
* Implemented conditional breakpoints.
Failed: 0, Passed: 74, Skipped: 189, Total: 263, Duration: 8 m 41 s - DebuggerTestSuite.dll (net6.0)
* Fix special characters and pointers.
Failed: 0, Passed: 116, Skipped: 177, Total: 293
* Fix merge
* Run debugger-tests on firefox using codespace
* Starting firefox correctly not stopping in the breakpoint yet.
* Remove unnecessary change
* Fix pause behavior (now showing correctly, pause on breakpoint, pause while stepping)
Start implementing evaluate expressions, working correctly on VSCode.
* Fix local tests.
* Fix missing )
* Passing 190 tests, evaluate expressions working.
* Remove Task.Delays.
Move some attributes from FirefoxMonoProxy to FirefoxExecutionContext.
* Fix container creation
* Trying to run firefox tests on CI.
* Moving file to the right place.
* Trying to run debugger-tests using firefox on CI.
* fixing path
* Missing url to download firefox on helix.
* On run the tests only on linux.
* Trying to download firefox on helix.
* fix error on helix-wasm.targets.
* trying to fix ci
* trying to install firefox on helix.
* Fixing firefox path
* Fix debugger tests on firefox
* fixing profile path
* Install libdbus-glib-1-2 on docker and on codespace
* Trying to run using firefox on CI
* update docker image
* Adding more messages to see errors on CI
* Trying to make it work on CI
* Real test on CI
* Trying to use the firefox machine only to run firefox tests
Retrying connection to Proxy
Remove extra messages added to help to fix CI
* Fix CI
* Fix CI
* Fix CI.
* Remove unnecessary changes.
* Using machine with sudo installed
* Addressing @lewing comments
* Fix run tests on codespace
Using image with python3
* Use default image to build and new image only to run firefox tests
* Fix unrelated change
* Fix ci
* check python version
* Print python versions
* Using image with PIP installed
* Using image with pip updated
* Remove unrelated changes
Increase time to wait for firefox to be ready
* Trying to fix evaluate tests.
* Fix evaluateoncallframe tests
* Trying to fix evaluation tests.
* trying to fix evaluateoncallframetests
* fiz evaluateoncallframetests
* Trying to kill firefox to avoid errors.
* Trying to fix EvaluateOnCallFrameTests
* Fix CI
* Remove failing test
* Fix misctests
* Fix other build errors.
* Trying to fix CI.
* Fix CI
* Remove unecessary message.
* Update src/tests/BuildWasmApps/Wasm.Debugger.Tests/Wasm.Debugger.Tests.csproj
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Merge error while accept @radical suggestion
* Merge error while accept @radical suggestion
* Update src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
Co-authored-by: Ankit Jain <radical@gmail.com>
* Apply suggestions from code review
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Abort the tcp connection if the proxy throws an exception
* Refactor a bit
* Use more compile time checks for chrome vs firefox
* fix pipeline
* Make debugger job names unique by including the browser
* fix runtime-wasm pipeline
* fix firefox ci job
* split into more files
* cleanup
* Add support for running chrome, and firefox tests in the same job
* fix yml
* fix build
* fix build
* fix windows build
* Don't delete profile folder nor pkill firefox
* Delete and create a new profile folder for each execution
* fix helix command line
* [wasm][debugger] Fix tests broken on 'main'
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
* wip
* big refactor
* chrome runs
* ff runs
* ff runs
* cleanup
* cleanup
* cleanup
* change console verbosity to info, for proxy
* More refactoring
* More refactoring, and fix some issues with connections, and other
cleanup
* some cleanup
* fix file name
* Improve cleanup after tests
* some refactoring, fixing some hangs, faster failures etc
* Fix BrowserCrash test for chrome
* fix up logging
* Improve error handling for the proxy running independently
* fix debugging from vscode
* proxy host: add --log-path for logs
* support canceling for the proxy host too, and distinguish different instances of the proxy
* Fix debugger after refreshing the debugged page.
* Fixing chrome debugging.
* Fix startup to work on chrome and also on firefox.
Co-authored-by: Ankit Jain <radical@gmail.com>
@ghostghost
locked as resolved and limited conversation to collaborators
May 14, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@tarekghtarekghtarekgh approved these changes

+1 more reviewer

@danmoseleydanmoseleydanmoseley left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@tarekghtarekgh

Labels

area-System.DateTimecommunity-contributionIndicates that the PR has been added by a community membernew-api-needs-documentation

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly


6 participants

@deeprobin@tarekgh@danmoseley@joperezr@ChristopherHaws@jeffhandley

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly by deeprobin · Pull Request #67666 · dotnet/runtime · GitHub
Skip to content
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly - #67666
Merged
tarekgh merged 15 commits into
dotnet:mainfrom 
deeprobin:issue-23799
Apr 14, 2022
Merged
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly#67666
tarekgh merged 15 commits into
dotnet:mainfrom 
deeprobin:issue-23799
Conversation
@deeprobin
@deeprobindeeprobin
commented
Apr 6, 2022

Copy link


Copy Markdown

Contributor
Proposal implementation of #23799 (closes#23799)
Proposal
namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}
Current state of implementation
  •  Proposal logic / implementation
  •  Ref Assembly
  •  Tests
/cc @tarekgh

/cc @ChristopherHaws
@ghostghost
added

community-contribution
Indicates that the PR has been added by a community member
new-api-needs-documentation

labels
Apr 6, 2022
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

Note regarding the new-api-needs-documentation label:
This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.
deeprobin
deeprobin
commented
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
@deeprobin
deeprobin
commented
Apr 6, 2022

Copy link


Copy Markdown

ContributorAuthor
It should be mentioned that I have taken the documentation comments from the current documentation and adapted them accordingly.
I have extended the current test cases accordingly. I assume that these are not flaky.
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-runtime

See info in area-owners.md if you want to be subscribed.
Issue Details
Proposal implementation of #23799 (closes #23799)
Proposal
namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}
Current state of implementation
  •  Proposal logic / implementation
  •  Ref Assembly
  •  Tests
/cc @tarekgh

/cc @ChristopherHaws
Author:deeprobin
Assignees:-
Labels:
area-System.Runtime, new-api-needs-documentation, community-contribution
Milestone:-
@tarekghtarekgh
self-assigned this
Apr 6, 2022
@tarekgh
tarekgh
commented
Apr 6, 2022

Copy link


Copy Markdown

Member
@deeprobin I'll try to take a look at this PR later today and will get back to you.
tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/TimeSpan.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

@danmoseley
danmoseley
commented
Apr 7, 2022

Copy link


Copy Markdown

Contributor
@deeprobin you might be interested to check that your tests are giving full code coverage of your code:
runtime/docs/workflow/building/libraries/code-coverage.md

Line 53
in
b248950
## Code coverage with System.Private.CoreLib code
@deeprobin
deeprobin
commented
Apr 7, 2022

Copy link


Copy Markdown

ContributorAuthor
@danmoseley

Thanks. I can take a look at that after the other open questions have been clarified here :).
tarekgh
tarekgh
reviewed
Apr 7, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeOffsetTests.cs
tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeTests.cs
@tarekgh
tarekgh
commented
Apr 12, 2022•

edited

Loading

Copy link


Copy Markdown

Member
Could you please add tests for TimeSpan new APIs? Also, when writing these tests please ensure using negative values in addition to positive values.
@tarekgh
tarekgh
commented
Apr 12, 2022

Copy link


Copy Markdown

Member
@deeprobin I left some comments I hope you can address them soon. After that we should be good to go. great work!
@joperezr
joperezr
commented
Apr 12, 2022•

edited

Loading

Copy link


Copy Markdown

Member
/azp run runtime (Rerunning CI in order to fetch changes merged in main which have fixed the Regex test)
@azure-pipelines
azure-pipelinesBot
commented
Apr 12, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@dotnetdotnet
deleted a comment from azure-pipelinesBotApr 12, 2022
danmoseley
danmoseley
reviewed
Apr 13, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

@tarekgh
tarekgh
commented
Apr 13, 2022

Copy link


Copy Markdown

Member
@deeprobin will be able to address the remaining feedback soon?
@deeprobin
deeprobin
requested a review
from tarekghApril 13, 2022 18:11
@tarekgh
tarekgh
commented
Apr 13, 2022

Copy link


Copy Markdown

Member
The failing test is unrelated and is tracked by the issue #67878
@tarekgh
tarekgh
merged commit b2ed250
into
dotnet:mainApr 14, 2022
@ChristopherHaws
ChristopherHaws
commented
Apr 14, 2022

Copy link


Copy Markdown

🎉 Thanks for implementing this @deeprobin!
@akoeplingerakoeplinger
mentioned this pull request
Apr 14, 2022
 Merged

@deeprobindeeprobin
mentioned this pull request
Apr 14, 2022
 Merged

radical added a commit
to radical/runtime
that referenced
this pull request
Apr 14, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
e87bda5
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
@radicalradical
mentioned this pull request
Apr 14, 2022
 Merged

radical added a commit
to thaystg/runtime
that referenced
this pull request
Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
79c9fde
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
radical added a commit
that referenced
this pull request
Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main' (#68054)
895715e
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
thaystg added a commit
that referenced
this pull request
Apr 26, 2022
@thaystg@radical
[wasm][debugger] Debug on firefox (#61776)
a970720
* First compiling and working version just proxying messages.
* almost working, already showing the cs files
* Working on firefox.
* Use internal e not public.
* Debugging on firefox working.
* Working after the merge
* Keep the TcpListener open and use a random port.
* Show null value.
* - Show JS Callstack
- Skip properties
- Get array value from evaluateAsyncJS and not use the preview value.
* Fix compilation
* Infrastructure to run debugger tests.
* fix merge
* run test.
* Skipping tests that are not passing on Firefox.
* Skipping tests that are not passing on Firefox.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Failed: 0, Passed: 39, Skipped: 203, Total: 242, Duration: 5 m 6 s - DebuggerTestSuite.dll (net6.0)
* Failed: 0, Passed: 66, Skipped: 195, Total: 261, Duration: 9 m 29 s - DebuggerTestSuite.dll (net6.0)
* Using ConditionalTheory and ConditionalFact implemented by @radical.
* Fixing side effect.
* Implemented conditional breakpoints.
Failed: 0, Passed: 74, Skipped: 189, Total: 263, Duration: 8 m 41 s - DebuggerTestSuite.dll (net6.0)
* Fix special characters and pointers.
Failed: 0, Passed: 116, Skipped: 177, Total: 293
* Fix merge
* Run debugger-tests on firefox using codespace
* Starting firefox correctly not stopping in the breakpoint yet.
* Remove unnecessary change
* Fix pause behavior (now showing correctly, pause on breakpoint, pause while stepping)
Start implementing evaluate expressions, working correctly on VSCode.
* Fix local tests.
* Fix missing )
* Passing 190 tests, evaluate expressions working.
* Remove Task.Delays.
Move some attributes from FirefoxMonoProxy to FirefoxExecutionContext.
* Fix container creation
* Trying to run firefox tests on CI.
* Moving file to the right place.
* Trying to run debugger-tests using firefox on CI.
* fixing path
* Missing url to download firefox on helix.
* On run the tests only on linux.
* Trying to download firefox on helix.
* fix error on helix-wasm.targets.
* trying to fix ci
* trying to install firefox on helix.
* Fixing firefox path
* Fix debugger tests on firefox
* fixing profile path
* Install libdbus-glib-1-2 on docker and on codespace
* Trying to run using firefox on CI
* update docker image
* Adding more messages to see errors on CI
* Trying to make it work on CI
* Real test on CI
* Trying to use the firefox machine only to run firefox tests
Retrying connection to Proxy
Remove extra messages added to help to fix CI
* Fix CI
* Fix CI
* Fix CI.
* Remove unnecessary changes.
* Using machine with sudo installed
* Addressing @lewing comments
* Fix run tests on codespace
Using image with python3
* Use default image to build and new image only to run firefox tests
* Fix unrelated change
* Fix ci
* check python version
* Print python versions
* Using image with PIP installed
* Using image with pip updated
* Remove unrelated changes
Increase time to wait for firefox to be ready
* Trying to fix evaluate tests.
* Fix evaluateoncallframe tests
* Trying to fix evaluation tests.
* trying to fix evaluateoncallframetests
* fiz evaluateoncallframetests
* Trying to kill firefox to avoid errors.
* Trying to fix EvaluateOnCallFrameTests
* Fix CI
* Remove failing test
* Fix misctests
* Fix other build errors.
* Trying to fix CI.
* Fix CI
* Remove unecessary message.
* Update src/tests/BuildWasmApps/Wasm.Debugger.Tests/Wasm.Debugger.Tests.csproj
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Merge error while accept @radical suggestion
* Merge error while accept @radical suggestion
* Update src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
Co-authored-by: Ankit Jain <radical@gmail.com>
* Apply suggestions from code review
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Abort the tcp connection if the proxy throws an exception
* Refactor a bit
* Use more compile time checks for chrome vs firefox
* fix pipeline
* Make debugger job names unique by including the browser
* fix runtime-wasm pipeline
* fix firefox ci job
* split into more files
* cleanup
* Add support for running chrome, and firefox tests in the same job
* fix yml
* fix build
* fix build
* fix windows build
* Don't delete profile folder nor pkill firefox
* Delete and create a new profile folder for each execution
* fix helix command line
* [wasm][debugger] Fix tests broken on 'main'
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
* wip
* big refactor
* chrome runs
* ff runs
* ff runs
* cleanup
* cleanup
* cleanup
* change console verbosity to info, for proxy
* More refactoring
* More refactoring, and fix some issues with connections, and other
cleanup
* some cleanup
* fix file name
* Improve cleanup after tests
* some refactoring, fixing some hangs, faster failures etc
* Fix BrowserCrash test for chrome
* fix up logging
* Improve error handling for the proxy running independently
* fix debugging from vscode
* proxy host: add --log-path for logs
* support canceling for the proxy host too, and distinguish different instances of the proxy
* Fix debugger after refreshing the debugged page.
* Fixing chrome debugging.
* Fix startup to work on chrome and also on firefox.
Co-authored-by: Ankit Jain <radical@gmail.com>
@ghostghost
locked as resolved and limited conversation to collaborators
May 14, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@tarekghtarekghtarekgh approved these changes

+1 more reviewer

@danmoseleydanmoseleydanmoseley left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@tarekghtarekgh

Labels

area-System.DateTimecommunity-contributionIndicates that the PR has been added by a community membernew-api-needs-documentation

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly


6 participants

@deeprobin@tarekgh@danmoseley@joperezr@ChristopherHaws@jeffhandley

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly by deeprobin · Pull Request #67666 · dotnet/runtime · GitHub
Skip to content
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly - #67666
Merged
tarekgh merged 15 commits into
dotnet:mainfrom 
deeprobin:issue-23799
Apr 14, 2022
Merged
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly#67666
tarekgh merged 15 commits into
dotnet:mainfrom 
deeprobin:issue-23799
Conversation
@deeprobin
@deeprobindeeprobin
commented
Apr 6, 2022

Copy link


Copy Markdown

Contributor
Proposal implementation of #23799 (closes#23799)
Proposal
namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}
Current state of implementation
  •  Proposal logic / implementation
  •  Ref Assembly
  •  Tests
/cc @tarekgh

/cc @ChristopherHaws
@ghostghost
added

community-contribution
Indicates that the PR has been added by a community member
new-api-needs-documentation

labels
Apr 6, 2022
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

Note regarding the new-api-needs-documentation label:
This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.
deeprobin
deeprobin
commented
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
@deeprobin
deeprobin
commented
Apr 6, 2022

Copy link


Copy Markdown

ContributorAuthor
It should be mentioned that I have taken the documentation comments from the current documentation and adapted them accordingly.
I have extended the current test cases accordingly. I assume that these are not flaky.
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-runtime

See info in area-owners.md if you want to be subscribed.
Issue Details
Proposal implementation of #23799 (closes #23799)
Proposal
namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}
Current state of implementation
  •  Proposal logic / implementation
  •  Ref Assembly
  •  Tests
/cc @tarekgh

/cc @ChristopherHaws
Author:deeprobin
Assignees:-
Labels:
area-System.Runtime, new-api-needs-documentation, community-contribution
Milestone:-
@tarekghtarekgh
self-assigned this
Apr 6, 2022
@tarekgh
tarekgh
commented
Apr 6, 2022

Copy link


Copy Markdown

Member
@deeprobin I'll try to take a look at this PR later today and will get back to you.
tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/TimeSpan.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

@danmoseley
danmoseley
commented
Apr 7, 2022

Copy link


Copy Markdown

Contributor
@deeprobin you might be interested to check that your tests are giving full code coverage of your code:
runtime/docs/workflow/building/libraries/code-coverage.md

Line 53
in
b248950
## Code coverage with System.Private.CoreLib code
@deeprobin
deeprobin
commented
Apr 7, 2022

Copy link


Copy Markdown

ContributorAuthor
@danmoseley

Thanks. I can take a look at that after the other open questions have been clarified here :).
tarekgh
tarekgh
reviewed
Apr 7, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeOffsetTests.cs
tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeTests.cs
@tarekgh
tarekgh
commented
Apr 12, 2022•

edited

Loading

Copy link


Copy Markdown

Member
Could you please add tests for TimeSpan new APIs? Also, when writing these tests please ensure using negative values in addition to positive values.
@tarekgh
tarekgh
commented
Apr 12, 2022

Copy link


Copy Markdown

Member
@deeprobin I left some comments I hope you can address them soon. After that we should be good to go. great work!
@joperezr
joperezr
commented
Apr 12, 2022•

edited

Loading

Copy link


Copy Markdown

Member
/azp run runtime (Rerunning CI in order to fetch changes merged in main which have fixed the Regex test)
@azure-pipelines
azure-pipelinesBot
commented
Apr 12, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@dotnetdotnet
deleted a comment from azure-pipelinesBotApr 12, 2022
danmoseley
danmoseley
reviewed
Apr 13, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

@tarekgh
tarekgh
commented
Apr 13, 2022

Copy link


Copy Markdown

Member
@deeprobin will be able to address the remaining feedback soon?
@deeprobin
deeprobin
requested a review
from tarekghApril 13, 2022 18:11
@tarekgh
tarekgh
commented
Apr 13, 2022

Copy link


Copy Markdown

Member
The failing test is unrelated and is tracked by the issue #67878
@tarekgh
tarekgh
merged commit b2ed250
into
dotnet:mainApr 14, 2022
@ChristopherHaws
ChristopherHaws
commented
Apr 14, 2022

Copy link


Copy Markdown

🎉 Thanks for implementing this @deeprobin!
@akoeplingerakoeplinger
mentioned this pull request
Apr 14, 2022
 Merged

@deeprobindeeprobin
mentioned this pull request
Apr 14, 2022
 Merged

radical added a commit
to radical/runtime
that referenced
this pull request
Apr 14, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
e87bda5
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
@radicalradical
mentioned this pull request
Apr 14, 2022
 Merged

radical added a commit
to thaystg/runtime
that referenced
this pull request
Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
79c9fde
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
radical added a commit
that referenced
this pull request
Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main' (#68054)
895715e
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
thaystg added a commit
that referenced
this pull request
Apr 26, 2022
@thaystg@radical
[wasm][debugger] Debug on firefox (#61776)
a970720
* First compiling and working version just proxying messages.
* almost working, already showing the cs files
* Working on firefox.
* Use internal e not public.
* Debugging on firefox working.
* Working after the merge
* Keep the TcpListener open and use a random port.
* Show null value.
* - Show JS Callstack
- Skip properties
- Get array value from evaluateAsyncJS and not use the preview value.
* Fix compilation
* Infrastructure to run debugger tests.
* fix merge
* run test.
* Skipping tests that are not passing on Firefox.
* Skipping tests that are not passing on Firefox.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Failed: 0, Passed: 39, Skipped: 203, Total: 242, Duration: 5 m 6 s - DebuggerTestSuite.dll (net6.0)
* Failed: 0, Passed: 66, Skipped: 195, Total: 261, Duration: 9 m 29 s - DebuggerTestSuite.dll (net6.0)
* Using ConditionalTheory and ConditionalFact implemented by @radical.
* Fixing side effect.
* Implemented conditional breakpoints.
Failed: 0, Passed: 74, Skipped: 189, Total: 263, Duration: 8 m 41 s - DebuggerTestSuite.dll (net6.0)
* Fix special characters and pointers.
Failed: 0, Passed: 116, Skipped: 177, Total: 293
* Fix merge
* Run debugger-tests on firefox using codespace
* Starting firefox correctly not stopping in the breakpoint yet.
* Remove unnecessary change
* Fix pause behavior (now showing correctly, pause on breakpoint, pause while stepping)
Start implementing evaluate expressions, working correctly on VSCode.
* Fix local tests.
* Fix missing )
* Passing 190 tests, evaluate expressions working.
* Remove Task.Delays.
Move some attributes from FirefoxMonoProxy to FirefoxExecutionContext.
* Fix container creation
* Trying to run firefox tests on CI.
* Moving file to the right place.
* Trying to run debugger-tests using firefox on CI.
* fixing path
* Missing url to download firefox on helix.
* On run the tests only on linux.
* Trying to download firefox on helix.
* fix error on helix-wasm.targets.
* trying to fix ci
* trying to install firefox on helix.
* Fixing firefox path
* Fix debugger tests on firefox
* fixing profile path
* Install libdbus-glib-1-2 on docker and on codespace
* Trying to run using firefox on CI
* update docker image
* Adding more messages to see errors on CI
* Trying to make it work on CI
* Real test on CI
* Trying to use the firefox machine only to run firefox tests
Retrying connection to Proxy
Remove extra messages added to help to fix CI
* Fix CI
* Fix CI
* Fix CI.
* Remove unnecessary changes.
* Using machine with sudo installed
* Addressing @lewing comments
* Fix run tests on codespace
Using image with python3
* Use default image to build and new image only to run firefox tests
* Fix unrelated change
* Fix ci
* check python version
* Print python versions
* Using image with PIP installed
* Using image with pip updated
* Remove unrelated changes
Increase time to wait for firefox to be ready
* Trying to fix evaluate tests.
* Fix evaluateoncallframe tests
* Trying to fix evaluation tests.
* trying to fix evaluateoncallframetests
* fiz evaluateoncallframetests
* Trying to kill firefox to avoid errors.
* Trying to fix EvaluateOnCallFrameTests
* Fix CI
* Remove failing test
* Fix misctests
* Fix other build errors.
* Trying to fix CI.
* Fix CI
* Remove unecessary message.
* Update src/tests/BuildWasmApps/Wasm.Debugger.Tests/Wasm.Debugger.Tests.csproj
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Merge error while accept @radical suggestion
* Merge error while accept @radical suggestion
* Update src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
Co-authored-by: Ankit Jain <radical@gmail.com>
* Apply suggestions from code review
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Abort the tcp connection if the proxy throws an exception
* Refactor a bit
* Use more compile time checks for chrome vs firefox
* fix pipeline
* Make debugger job names unique by including the browser
* fix runtime-wasm pipeline
* fix firefox ci job
* split into more files
* cleanup
* Add support for running chrome, and firefox tests in the same job
* fix yml
* fix build
* fix build
* fix windows build
* Don't delete profile folder nor pkill firefox
* Delete and create a new profile folder for each execution
* fix helix command line
* [wasm][debugger] Fix tests broken on 'main'
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
* wip
* big refactor
* chrome runs
* ff runs
* ff runs
* cleanup
* cleanup
* cleanup
* change console verbosity to info, for proxy
* More refactoring
* More refactoring, and fix some issues with connections, and other
cleanup
* some cleanup
* fix file name
* Improve cleanup after tests
* some refactoring, fixing some hangs, faster failures etc
* Fix BrowserCrash test for chrome
* fix up logging
* Improve error handling for the proxy running independently
* fix debugging from vscode
* proxy host: add --log-path for logs
* support canceling for the proxy host too, and distinguish different instances of the proxy
* Fix debugger after refreshing the debugged page.
* Fixing chrome debugging.
* Fix startup to work on chrome and also on firefox.
Co-authored-by: Ankit Jain <radical@gmail.com>
@ghostghost
locked as resolved and limited conversation to collaborators
May 14, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@tarekghtarekghtarekgh approved these changes

+1 more reviewer

@danmoseleydanmoseleydanmoseley left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@tarekghtarekgh

Labels

area-System.DateTimecommunity-contributionIndicates that the PR has been added by a community membernew-api-needs-documentation

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly


6 participants

@deeprobin@tarekgh@danmoseley@joperezr@ChristopherHaws@jeffhandley

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly by deeprobin · Pull Request #67666 · dotnet/runtime · GitHub
Skip to content
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly - #67666
Merged
tarekgh merged 15 commits into
dotnet:mainfrom 
deeprobin:issue-23799
Apr 14, 2022
Merged
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly#67666
tarekgh merged 15 commits into
dotnet:mainfrom 
deeprobin:issue-23799
Conversation
@deeprobin
@deeprobindeeprobin
commented
Apr 6, 2022

Copy link


Copy Markdown

Contributor
Proposal implementation of #23799 (closes#23799)
Proposal
namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}
Current state of implementation
  •  Proposal logic / implementation
  •  Ref Assembly
  •  Tests
/cc @tarekgh

/cc @ChristopherHaws
@ghostghost
added

community-contribution
Indicates that the PR has been added by a community member
new-api-needs-documentation

labels
Apr 6, 2022
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

Note regarding the new-api-needs-documentation label:
This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.
deeprobin
deeprobin
commented
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
@deeprobin
deeprobin
commented
Apr 6, 2022

Copy link


Copy Markdown

ContributorAuthor
It should be mentioned that I have taken the documentation comments from the current documentation and adapted them accordingly.
I have extended the current test cases accordingly. I assume that these are not flaky.
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-runtime

See info in area-owners.md if you want to be subscribed.
Issue Details
Proposal implementation of #23799 (closes #23799)
Proposal
namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}
Current state of implementation
  •  Proposal logic / implementation
  •  Ref Assembly
  •  Tests
/cc @tarekgh

/cc @ChristopherHaws
Author:deeprobin
Assignees:-
Labels:
area-System.Runtime, new-api-needs-documentation, community-contribution
Milestone:-
@tarekghtarekgh
self-assigned this
Apr 6, 2022
@tarekgh
tarekgh
commented
Apr 6, 2022

Copy link


Copy Markdown

Member
@deeprobin I'll try to take a look at this PR later today and will get back to you.
tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/TimeSpan.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

@danmoseley
danmoseley
commented
Apr 7, 2022

Copy link


Copy Markdown

Contributor
@deeprobin you might be interested to check that your tests are giving full code coverage of your code:
runtime/docs/workflow/building/libraries/code-coverage.md

Line 53
in
b248950
## Code coverage with System.Private.CoreLib code
@deeprobin
deeprobin
commented
Apr 7, 2022

Copy link


Copy Markdown

ContributorAuthor
@danmoseley

Thanks. I can take a look at that after the other open questions have been clarified here :).
tarekgh
tarekgh
reviewed
Apr 7, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeOffsetTests.cs
tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeTests.cs
@tarekgh
tarekgh
commented
Apr 12, 2022•

edited

Loading

Copy link


Copy Markdown

Member
Could you please add tests for TimeSpan new APIs? Also, when writing these tests please ensure using negative values in addition to positive values.
@tarekgh
tarekgh
commented
Apr 12, 2022

Copy link


Copy Markdown

Member
@deeprobin I left some comments I hope you can address them soon. After that we should be good to go. great work!
@joperezr
joperezr
commented
Apr 12, 2022•

edited

Loading

Copy link


Copy Markdown

Member
/azp run runtime (Rerunning CI in order to fetch changes merged in main which have fixed the Regex test)
@azure-pipelines
azure-pipelinesBot
commented
Apr 12, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@dotnetdotnet
deleted a comment from azure-pipelinesBotApr 12, 2022
danmoseley
danmoseley
reviewed
Apr 13, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

@tarekgh
tarekgh
commented
Apr 13, 2022

Copy link


Copy Markdown

Member
@deeprobin will be able to address the remaining feedback soon?
@deeprobin
deeprobin
requested a review
from tarekghApril 13, 2022 18:11
@tarekgh
tarekgh
commented
Apr 13, 2022

Copy link


Copy Markdown

Member
The failing test is unrelated and is tracked by the issue #67878
@tarekgh
tarekgh
merged commit b2ed250
into
dotnet:mainApr 14, 2022
@ChristopherHaws
ChristopherHaws
commented
Apr 14, 2022

Copy link


Copy Markdown

🎉 Thanks for implementing this @deeprobin!
@akoeplingerakoeplinger
mentioned this pull request
Apr 14, 2022
 Merged

@deeprobindeeprobin
mentioned this pull request
Apr 14, 2022
 Merged

radical added a commit
to radical/runtime
that referenced
this pull request
Apr 14, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
e87bda5
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
@radicalradical
mentioned this pull request
Apr 14, 2022
 Merged

radical added a commit
to thaystg/runtime
that referenced
this pull request
Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
79c9fde
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
radical added a commit
that referenced
this pull request
Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main' (#68054)
895715e
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
thaystg added a commit
that referenced
this pull request
Apr 26, 2022
@thaystg@radical
[wasm][debugger] Debug on firefox (#61776)
a970720
* First compiling and working version just proxying messages.
* almost working, already showing the cs files
* Working on firefox.
* Use internal e not public.
* Debugging on firefox working.
* Working after the merge
* Keep the TcpListener open and use a random port.
* Show null value.
* - Show JS Callstack
- Skip properties
- Get array value from evaluateAsyncJS and not use the preview value.
* Fix compilation
* Infrastructure to run debugger tests.
* fix merge
* run test.
* Skipping tests that are not passing on Firefox.
* Skipping tests that are not passing on Firefox.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Failed: 0, Passed: 39, Skipped: 203, Total: 242, Duration: 5 m 6 s - DebuggerTestSuite.dll (net6.0)
* Failed: 0, Passed: 66, Skipped: 195, Total: 261, Duration: 9 m 29 s - DebuggerTestSuite.dll (net6.0)
* Using ConditionalTheory and ConditionalFact implemented by @radical.
* Fixing side effect.
* Implemented conditional breakpoints.
Failed: 0, Passed: 74, Skipped: 189, Total: 263, Duration: 8 m 41 s - DebuggerTestSuite.dll (net6.0)
* Fix special characters and pointers.
Failed: 0, Passed: 116, Skipped: 177, Total: 293
* Fix merge
* Run debugger-tests on firefox using codespace
* Starting firefox correctly not stopping in the breakpoint yet.
* Remove unnecessary change
* Fix pause behavior (now showing correctly, pause on breakpoint, pause while stepping)
Start implementing evaluate expressions, working correctly on VSCode.
* Fix local tests.
* Fix missing )
* Passing 190 tests, evaluate expressions working.
* Remove Task.Delays.
Move some attributes from FirefoxMonoProxy to FirefoxExecutionContext.
* Fix container creation
* Trying to run firefox tests on CI.
* Moving file to the right place.
* Trying to run debugger-tests using firefox on CI.
* fixing path
* Missing url to download firefox on helix.
* On run the tests only on linux.
* Trying to download firefox on helix.
* fix error on helix-wasm.targets.
* trying to fix ci
* trying to install firefox on helix.
* Fixing firefox path
* Fix debugger tests on firefox
* fixing profile path
* Install libdbus-glib-1-2 on docker and on codespace
* Trying to run using firefox on CI
* update docker image
* Adding more messages to see errors on CI
* Trying to make it work on CI
* Real test on CI
* Trying to use the firefox machine only to run firefox tests
Retrying connection to Proxy
Remove extra messages added to help to fix CI
* Fix CI
* Fix CI
* Fix CI.
* Remove unnecessary changes.
* Using machine with sudo installed
* Addressing @lewing comments
* Fix run tests on codespace
Using image with python3
* Use default image to build and new image only to run firefox tests
* Fix unrelated change
* Fix ci
* check python version
* Print python versions
* Using image with PIP installed
* Using image with pip updated
* Remove unrelated changes
Increase time to wait for firefox to be ready
* Trying to fix evaluate tests.
* Fix evaluateoncallframe tests
* Trying to fix evaluation tests.
* trying to fix evaluateoncallframetests
* fiz evaluateoncallframetests
* Trying to kill firefox to avoid errors.
* Trying to fix EvaluateOnCallFrameTests
* Fix CI
* Remove failing test
* Fix misctests
* Fix other build errors.
* Trying to fix CI.
* Fix CI
* Remove unecessary message.
* Update src/tests/BuildWasmApps/Wasm.Debugger.Tests/Wasm.Debugger.Tests.csproj
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Merge error while accept @radical suggestion
* Merge error while accept @radical suggestion
* Update src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
Co-authored-by: Ankit Jain <radical@gmail.com>
* Apply suggestions from code review
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Abort the tcp connection if the proxy throws an exception
* Refactor a bit
* Use more compile time checks for chrome vs firefox
* fix pipeline
* Make debugger job names unique by including the browser
* fix runtime-wasm pipeline
* fix firefox ci job
* split into more files
* cleanup
* Add support for running chrome, and firefox tests in the same job
* fix yml
* fix build
* fix build
* fix windows build
* Don't delete profile folder nor pkill firefox
* Delete and create a new profile folder for each execution
* fix helix command line
* [wasm][debugger] Fix tests broken on 'main'
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
* wip
* big refactor
* chrome runs
* ff runs
* ff runs
* cleanup
* cleanup
* cleanup
* change console verbosity to info, for proxy
* More refactoring
* More refactoring, and fix some issues with connections, and other
cleanup
* some cleanup
* fix file name
* Improve cleanup after tests
* some refactoring, fixing some hangs, faster failures etc
* Fix BrowserCrash test for chrome
* fix up logging
* Improve error handling for the proxy running independently
* fix debugging from vscode
* proxy host: add --log-path for logs
* support canceling for the proxy host too, and distinguish different instances of the proxy
* Fix debugger after refreshing the debugged page.
* Fixing chrome debugging.
* Fix startup to work on chrome and also on firefox.
Co-authored-by: Ankit Jain <radical@gmail.com>
@ghostghost
locked as resolved and limited conversation to collaborators
May 14, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@tarekghtarekghtarekgh approved these changes

+1 more reviewer

@danmoseleydanmoseleydanmoseley left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@tarekghtarekgh

Labels

area-System.DateTimecommunity-contributionIndicates that the PR has been added by a community membernew-api-needs-documentation

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly


6 participants

@deeprobin@tarekgh@danmoseley@joperezr@ChristopherHaws@jeffhandley

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly by deeprobin · Pull Request #67666 · dotnet/runtime · GitHub
Skip to content
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly - #67666
Merged
tarekgh merged 15 commits into
dotnet:mainfrom 
deeprobin:issue-23799
Apr 14, 2022
Merged
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly#67666
tarekgh merged 15 commits into
dotnet:mainfrom 
deeprobin:issue-23799
Conversation
@deeprobin
@deeprobindeeprobin
commented
Apr 6, 2022

Copy link


Copy Markdown

Contributor
Proposal implementation of #23799 (closes#23799)
Proposal
namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}
Current state of implementation
  •  Proposal logic / implementation
  •  Ref Assembly
  •  Tests
/cc @tarekgh

/cc @ChristopherHaws
@ghostghost
added

community-contribution
Indicates that the PR has been added by a community member
new-api-needs-documentation

labels
Apr 6, 2022
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

Note regarding the new-api-needs-documentation label:
This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.
deeprobin
deeprobin
commented
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
@deeprobin
deeprobin
commented
Apr 6, 2022

Copy link


Copy Markdown

ContributorAuthor
It should be mentioned that I have taken the documentation comments from the current documentation and adapted them accordingly.
I have extended the current test cases accordingly. I assume that these are not flaky.
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-runtime

See info in area-owners.md if you want to be subscribed.
Issue Details
Proposal implementation of #23799 (closes #23799)
Proposal
namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}
Current state of implementation
  •  Proposal logic / implementation
  •  Ref Assembly
  •  Tests
/cc @tarekgh

/cc @ChristopherHaws
Author:deeprobin
Assignees:-
Labels:
area-System.Runtime, new-api-needs-documentation, community-contribution
Milestone:-
@tarekghtarekgh
self-assigned this
Apr 6, 2022
@tarekgh
tarekgh
commented
Apr 6, 2022

Copy link


Copy Markdown

Member
@deeprobin I'll try to take a look at this PR later today and will get back to you.
tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/TimeSpan.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

@danmoseley
danmoseley
commented
Apr 7, 2022

Copy link


Copy Markdown

Contributor
@deeprobin you might be interested to check that your tests are giving full code coverage of your code:
runtime/docs/workflow/building/libraries/code-coverage.md

Line 53
in
b248950
## Code coverage with System.Private.CoreLib code
@deeprobin
deeprobin
commented
Apr 7, 2022

Copy link


Copy Markdown

ContributorAuthor
@danmoseley

Thanks. I can take a look at that after the other open questions have been clarified here :).
tarekgh
tarekgh
reviewed
Apr 7, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeOffsetTests.cs
tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeTests.cs
@tarekgh
tarekgh
commented
Apr 12, 2022•

edited

Loading

Copy link


Copy Markdown

Member
Could you please add tests for TimeSpan new APIs? Also, when writing these tests please ensure using negative values in addition to positive values.
@tarekgh
tarekgh
commented
Apr 12, 2022

Copy link


Copy Markdown

Member
@deeprobin I left some comments I hope you can address them soon. After that we should be good to go. great work!
@joperezr
joperezr
commented
Apr 12, 2022•

edited

Loading

Copy link


Copy Markdown

Member
/azp run runtime (Rerunning CI in order to fetch changes merged in main which have fixed the Regex test)
@azure-pipelines
azure-pipelinesBot
commented
Apr 12, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@dotnetdotnet
deleted a comment from azure-pipelinesBotApr 12, 2022
danmoseley
danmoseley
reviewed
Apr 13, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

@tarekgh
tarekgh
commented
Apr 13, 2022

Copy link


Copy Markdown

Member
@deeprobin will be able to address the remaining feedback soon?
@deeprobin
deeprobin
requested a review
from tarekghApril 13, 2022 18:11
@tarekgh
tarekgh
commented
Apr 13, 2022

Copy link


Copy Markdown

Member
The failing test is unrelated and is tracked by the issue #67878
@tarekgh
tarekgh
merged commit b2ed250
into
dotnet:mainApr 14, 2022
@ChristopherHaws
ChristopherHaws
commented
Apr 14, 2022

Copy link


Copy Markdown

🎉 Thanks for implementing this @deeprobin!
@akoeplingerakoeplinger
mentioned this pull request
Apr 14, 2022
 Merged

@deeprobindeeprobin
mentioned this pull request
Apr 14, 2022
 Merged

radical added a commit
to radical/runtime
that referenced
this pull request
Apr 14, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
e87bda5
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
@radicalradical
mentioned this pull request
Apr 14, 2022
 Merged

radical added a commit
to thaystg/runtime
that referenced
this pull request
Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
79c9fde
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
radical added a commit
that referenced
this pull request
Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main' (#68054)
895715e
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
thaystg added a commit
that referenced
this pull request
Apr 26, 2022
@thaystg@radical
[wasm][debugger] Debug on firefox (#61776)
a970720
* First compiling and working version just proxying messages.
* almost working, already showing the cs files
* Working on firefox.
* Use internal e not public.
* Debugging on firefox working.
* Working after the merge
* Keep the TcpListener open and use a random port.
* Show null value.
* - Show JS Callstack
- Skip properties
- Get array value from evaluateAsyncJS and not use the preview value.
* Fix compilation
* Infrastructure to run debugger tests.
* fix merge
* run test.
* Skipping tests that are not passing on Firefox.
* Skipping tests that are not passing on Firefox.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Failed: 0, Passed: 39, Skipped: 203, Total: 242, Duration: 5 m 6 s - DebuggerTestSuite.dll (net6.0)
* Failed: 0, Passed: 66, Skipped: 195, Total: 261, Duration: 9 m 29 s - DebuggerTestSuite.dll (net6.0)
* Using ConditionalTheory and ConditionalFact implemented by @radical.
* Fixing side effect.
* Implemented conditional breakpoints.
Failed: 0, Passed: 74, Skipped: 189, Total: 263, Duration: 8 m 41 s - DebuggerTestSuite.dll (net6.0)
* Fix special characters and pointers.
Failed: 0, Passed: 116, Skipped: 177, Total: 293
* Fix merge
* Run debugger-tests on firefox using codespace
* Starting firefox correctly not stopping in the breakpoint yet.
* Remove unnecessary change
* Fix pause behavior (now showing correctly, pause on breakpoint, pause while stepping)
Start implementing evaluate expressions, working correctly on VSCode.
* Fix local tests.
* Fix missing )
* Passing 190 tests, evaluate expressions working.
* Remove Task.Delays.
Move some attributes from FirefoxMonoProxy to FirefoxExecutionContext.
* Fix container creation
* Trying to run firefox tests on CI.
* Moving file to the right place.
* Trying to run debugger-tests using firefox on CI.
* fixing path
* Missing url to download firefox on helix.
* On run the tests only on linux.
* Trying to download firefox on helix.
* fix error on helix-wasm.targets.
* trying to fix ci
* trying to install firefox on helix.
* Fixing firefox path
* Fix debugger tests on firefox
* fixing profile path
* Install libdbus-glib-1-2 on docker and on codespace
* Trying to run using firefox on CI
* update docker image
* Adding more messages to see errors on CI
* Trying to make it work on CI
* Real test on CI
* Trying to use the firefox machine only to run firefox tests
Retrying connection to Proxy
Remove extra messages added to help to fix CI
* Fix CI
* Fix CI
* Fix CI.
* Remove unnecessary changes.
* Using machine with sudo installed
* Addressing @lewing comments
* Fix run tests on codespace
Using image with python3
* Use default image to build and new image only to run firefox tests
* Fix unrelated change
* Fix ci
* check python version
* Print python versions
* Using image with PIP installed
* Using image with pip updated
* Remove unrelated changes
Increase time to wait for firefox to be ready
* Trying to fix evaluate tests.
* Fix evaluateoncallframe tests
* Trying to fix evaluation tests.
* trying to fix evaluateoncallframetests
* fiz evaluateoncallframetests
* Trying to kill firefox to avoid errors.
* Trying to fix EvaluateOnCallFrameTests
* Fix CI
* Remove failing test
* Fix misctests
* Fix other build errors.
* Trying to fix CI.
* Fix CI
* Remove unecessary message.
* Update src/tests/BuildWasmApps/Wasm.Debugger.Tests/Wasm.Debugger.Tests.csproj
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Merge error while accept @radical suggestion
* Merge error while accept @radical suggestion
* Update src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
Co-authored-by: Ankit Jain <radical@gmail.com>
* Apply suggestions from code review
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Abort the tcp connection if the proxy throws an exception
* Refactor a bit
* Use more compile time checks for chrome vs firefox
* fix pipeline
* Make debugger job names unique by including the browser
* fix runtime-wasm pipeline
* fix firefox ci job
* split into more files
* cleanup
* Add support for running chrome, and firefox tests in the same job
* fix yml
* fix build
* fix build
* fix windows build
* Don't delete profile folder nor pkill firefox
* Delete and create a new profile folder for each execution
* fix helix command line
* [wasm][debugger] Fix tests broken on 'main'
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
* wip
* big refactor
* chrome runs
* ff runs
* ff runs
* cleanup
* cleanup
* cleanup
* change console verbosity to info, for proxy
* More refactoring
* More refactoring, and fix some issues with connections, and other
cleanup
* some cleanup
* fix file name
* Improve cleanup after tests
* some refactoring, fixing some hangs, faster failures etc
* Fix BrowserCrash test for chrome
* fix up logging
* Improve error handling for the proxy running independently
* fix debugging from vscode
* proxy host: add --log-path for logs
* support canceling for the proxy host too, and distinguish different instances of the proxy
* Fix debugger after refreshing the debugged page.
* Fixing chrome debugging.
* Fix startup to work on chrome and also on firefox.
Co-authored-by: Ankit Jain <radical@gmail.com>
@ghostghost
locked as resolved and limited conversation to collaborators
May 14, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@tarekghtarekghtarekgh approved these changes

+1 more reviewer

@danmoseleydanmoseleydanmoseley left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@tarekghtarekgh

Labels

area-System.DateTimecommunity-contributionIndicates that the PR has been added by a community membernew-api-needs-documentation

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly


6 participants

@deeprobin@tarekgh@danmoseley@joperezr@ChristopherHaws@jeffhandley

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly by deeprobin · Pull Request #67666 · dotnet/runtime · GitHub
Skip to content
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly - #67666
Merged
tarekgh merged 15 commits into
dotnet:mainfrom 
deeprobin:issue-23799
Apr 14, 2022
Merged
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly#67666
tarekgh merged 15 commits into
dotnet:mainfrom 
deeprobin:issue-23799
Conversation
@deeprobin
@deeprobindeeprobin
commented
Apr 6, 2022

Copy link


Copy Markdown

Contributor
Proposal implementation of #23799 (closes#23799)
Proposal
namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}
Current state of implementation
  •  Proposal logic / implementation
  •  Ref Assembly
  •  Tests
/cc @tarekgh

/cc @ChristopherHaws
@ghostghost
added

community-contribution
Indicates that the PR has been added by a community member
new-api-needs-documentation

labels
Apr 6, 2022
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

Note regarding the new-api-needs-documentation label:
This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.
deeprobin
deeprobin
commented
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
@deeprobin
deeprobin
commented
Apr 6, 2022

Copy link


Copy Markdown

ContributorAuthor
It should be mentioned that I have taken the documentation comments from the current documentation and adapted them accordingly.
I have extended the current test cases accordingly. I assume that these are not flaky.
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-runtime

See info in area-owners.md if you want to be subscribed.
Issue Details
Proposal implementation of #23799 (closes #23799)
Proposal
namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}
Current state of implementation
  •  Proposal logic / implementation
  •  Ref Assembly
  •  Tests
/cc @tarekgh

/cc @ChristopherHaws
Author:deeprobin
Assignees:-
Labels:
area-System.Runtime, new-api-needs-documentation, community-contribution
Milestone:-
@tarekghtarekgh
self-assigned this
Apr 6, 2022
@tarekgh
tarekgh
commented
Apr 6, 2022

Copy link


Copy Markdown

Member
@deeprobin I'll try to take a look at this PR later today and will get back to you.
tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/TimeSpan.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

@danmoseley
danmoseley
commented
Apr 7, 2022

Copy link


Copy Markdown

Contributor
@deeprobin you might be interested to check that your tests are giving full code coverage of your code:
runtime/docs/workflow/building/libraries/code-coverage.md

Line 53
in
b248950
## Code coverage with System.Private.CoreLib code
@deeprobin
deeprobin
commented
Apr 7, 2022

Copy link


Copy Markdown

ContributorAuthor
@danmoseley

Thanks. I can take a look at that after the other open questions have been clarified here :).
tarekgh
tarekgh
reviewed
Apr 7, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeOffsetTests.cs
tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeTests.cs
@tarekgh
tarekgh
commented
Apr 12, 2022•

edited

Loading

Copy link


Copy Markdown

Member
Could you please add tests for TimeSpan new APIs? Also, when writing these tests please ensure using negative values in addition to positive values.
@tarekgh
tarekgh
commented
Apr 12, 2022

Copy link


Copy Markdown

Member
@deeprobin I left some comments I hope you can address them soon. After that we should be good to go. great work!
@joperezr
joperezr
commented
Apr 12, 2022•

edited

Loading

Copy link


Copy Markdown

Member
/azp run runtime (Rerunning CI in order to fetch changes merged in main which have fixed the Regex test)
@azure-pipelines
azure-pipelinesBot
commented
Apr 12, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@dotnetdotnet
deleted a comment from azure-pipelinesBotApr 12, 2022
danmoseley
danmoseley
reviewed
Apr 13, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

@tarekgh
tarekgh
commented
Apr 13, 2022

Copy link


Copy Markdown

Member
@deeprobin will be able to address the remaining feedback soon?
@deeprobin
deeprobin
requested a review
from tarekghApril 13, 2022 18:11
@tarekgh
tarekgh
commented
Apr 13, 2022

Copy link


Copy Markdown

Member
The failing test is unrelated and is tracked by the issue #67878
@tarekgh
tarekgh
merged commit b2ed250
into
dotnet:mainApr 14, 2022
@ChristopherHaws
ChristopherHaws
commented
Apr 14, 2022

Copy link


Copy Markdown

🎉 Thanks for implementing this @deeprobin!
@akoeplingerakoeplinger
mentioned this pull request
Apr 14, 2022
 Merged

@deeprobindeeprobin
mentioned this pull request
Apr 14, 2022
 Merged

radical added a commit
to radical/runtime
that referenced
this pull request
Apr 14, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
e87bda5
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
@radicalradical
mentioned this pull request
Apr 14, 2022
 Merged

radical added a commit
to thaystg/runtime
that referenced
this pull request
Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
79c9fde
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
radical added a commit
that referenced
this pull request
Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main' (#68054)
895715e
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
thaystg added a commit
that referenced
this pull request
Apr 26, 2022
@thaystg@radical
[wasm][debugger] Debug on firefox (#61776)
a970720
* First compiling and working version just proxying messages.
* almost working, already showing the cs files
* Working on firefox.
* Use internal e not public.
* Debugging on firefox working.
* Working after the merge
* Keep the TcpListener open and use a random port.
* Show null value.
* - Show JS Callstack
- Skip properties
- Get array value from evaluateAsyncJS and not use the preview value.
* Fix compilation
* Infrastructure to run debugger tests.
* fix merge
* run test.
* Skipping tests that are not passing on Firefox.
* Skipping tests that are not passing on Firefox.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Failed: 0, Passed: 39, Skipped: 203, Total: 242, Duration: 5 m 6 s - DebuggerTestSuite.dll (net6.0)
* Failed: 0, Passed: 66, Skipped: 195, Total: 261, Duration: 9 m 29 s - DebuggerTestSuite.dll (net6.0)
* Using ConditionalTheory and ConditionalFact implemented by @radical.
* Fixing side effect.
* Implemented conditional breakpoints.
Failed: 0, Passed: 74, Skipped: 189, Total: 263, Duration: 8 m 41 s - DebuggerTestSuite.dll (net6.0)
* Fix special characters and pointers.
Failed: 0, Passed: 116, Skipped: 177, Total: 293
* Fix merge
* Run debugger-tests on firefox using codespace
* Starting firefox correctly not stopping in the breakpoint yet.
* Remove unnecessary change
* Fix pause behavior (now showing correctly, pause on breakpoint, pause while stepping)
Start implementing evaluate expressions, working correctly on VSCode.
* Fix local tests.
* Fix missing )
* Passing 190 tests, evaluate expressions working.
* Remove Task.Delays.
Move some attributes from FirefoxMonoProxy to FirefoxExecutionContext.
* Fix container creation
* Trying to run firefox tests on CI.
* Moving file to the right place.
* Trying to run debugger-tests using firefox on CI.
* fixing path
* Missing url to download firefox on helix.
* On run the tests only on linux.
* Trying to download firefox on helix.
* fix error on helix-wasm.targets.
* trying to fix ci
* trying to install firefox on helix.
* Fixing firefox path
* Fix debugger tests on firefox
* fixing profile path
* Install libdbus-glib-1-2 on docker and on codespace
* Trying to run using firefox on CI
* update docker image
* Adding more messages to see errors on CI
* Trying to make it work on CI
* Real test on CI
* Trying to use the firefox machine only to run firefox tests
Retrying connection to Proxy
Remove extra messages added to help to fix CI
* Fix CI
* Fix CI
* Fix CI.
* Remove unnecessary changes.
* Using machine with sudo installed
* Addressing @lewing comments
* Fix run tests on codespace
Using image with python3
* Use default image to build and new image only to run firefox tests
* Fix unrelated change
* Fix ci
* check python version
* Print python versions
* Using image with PIP installed
* Using image with pip updated
* Remove unrelated changes
Increase time to wait for firefox to be ready
* Trying to fix evaluate tests.
* Fix evaluateoncallframe tests
* Trying to fix evaluation tests.
* trying to fix evaluateoncallframetests
* fiz evaluateoncallframetests
* Trying to kill firefox to avoid errors.
* Trying to fix EvaluateOnCallFrameTests
* Fix CI
* Remove failing test
* Fix misctests
* Fix other build errors.
* Trying to fix CI.
* Fix CI
* Remove unecessary message.
* Update src/tests/BuildWasmApps/Wasm.Debugger.Tests/Wasm.Debugger.Tests.csproj
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Merge error while accept @radical suggestion
* Merge error while accept @radical suggestion
* Update src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
Co-authored-by: Ankit Jain <radical@gmail.com>
* Apply suggestions from code review
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Abort the tcp connection if the proxy throws an exception
* Refactor a bit
* Use more compile time checks for chrome vs firefox
* fix pipeline
* Make debugger job names unique by including the browser
* fix runtime-wasm pipeline
* fix firefox ci job
* split into more files
* cleanup
* Add support for running chrome, and firefox tests in the same job
* fix yml
* fix build
* fix build
* fix windows build
* Don't delete profile folder nor pkill firefox
* Delete and create a new profile folder for each execution
* fix helix command line
* [wasm][debugger] Fix tests broken on 'main'
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
* wip
* big refactor
* chrome runs
* ff runs
* ff runs
* cleanup
* cleanup
* cleanup
* change console verbosity to info, for proxy
* More refactoring
* More refactoring, and fix some issues with connections, and other
cleanup
* some cleanup
* fix file name
* Improve cleanup after tests
* some refactoring, fixing some hangs, faster failures etc
* Fix BrowserCrash test for chrome
* fix up logging
* Improve error handling for the proxy running independently
* fix debugging from vscode
* proxy host: add --log-path for logs
* support canceling for the proxy host too, and distinguish different instances of the proxy
* Fix debugger after refreshing the debugged page.
* Fixing chrome debugging.
* Fix startup to work on chrome and also on firefox.
Co-authored-by: Ankit Jain <radical@gmail.com>
@ghostghost
locked as resolved and limited conversation to collaborators
May 14, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@tarekghtarekghtarekgh approved these changes

+1 more reviewer

@danmoseleydanmoseleydanmoseley left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@tarekghtarekgh

Labels

area-System.DateTimecommunity-contributionIndicates that the PR has been added by a community membernew-api-needs-documentation

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly


6 participants

@deeprobin@tarekgh@danmoseley@joperezr@ChristopherHaws@jeffhandley

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly by deeprobin · Pull Request #67666 · dotnet/runtime · GitHub
Skip to content
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly - #67666
Merged
tarekgh merged 15 commits into
dotnet:mainfrom 
deeprobin:issue-23799
Apr 14, 2022
Merged
[API Implementation]: Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly#67666
tarekgh merged 15 commits into
dotnet:mainfrom 
deeprobin:issue-23799
Conversation
@deeprobin
@deeprobindeeprobin
commented
Apr 6, 2022

Copy link


Copy Markdown

Contributor
Proposal implementation of #23799 (closes#23799)
Proposal
namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}
Current state of implementation
  •  Proposal logic / implementation
  •  Ref Assembly
  •  Tests
/cc @tarekgh

/cc @ChristopherHaws
@ghostghost
added

community-contribution
Indicates that the PR has been added by a community member
new-api-needs-documentation

labels
Apr 6, 2022
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

Note regarding the new-api-needs-documentation label:
This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.
deeprobin
deeprobin
commented
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
@deeprobin
deeprobin
commented
Apr 6, 2022

Copy link


Copy Markdown

ContributorAuthor
It should be mentioned that I have taken the documentation comments from the current documentation and adapted them accordingly.
I have extended the current test cases accordingly. I assume that these are not flaky.
@ghost
ghost
commented
Apr 6, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-runtime

See info in area-owners.md if you want to be subscribed.
Issue Details
Proposal implementation of #23799 (closes #23799)
Proposal
namespaceSystem{publicstructDateTime{publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,DateTimeKindkind);publicDateTime(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeAddMicroseconds(doublevalue);}publicstructDateTimeOffset{publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset);publicDateTimeOffset(intyear,intmonth,intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond,TimeSpanoffset,Calendarcalendar);publicintMicrosecond{get;}publicintNanosecond{get;}publicDateTimeOffsetAddMicroseconds(doublevalue);}publicstructTimeSpan{publicconstlongTicksPerMicrosecond;publicconstlongNanosecondsPerTick;publicTimeSpan(intdays,inthours,intminutes,intseconds,intmilliseconds,intmicroseconds);publicintMicroseconds{get;}publicintNanoseconds{get;}publicdoubleTotalMicroseconds{get;}publicdoubleTotalNanoseconds{get;}publicstaticTimeSpanFromMicroseconds(doublevalue);}publicstructTimeOnly{publicTimeOnly(intday,inthour,intminute,intsecond,intmillisecond,intmicrosecond);publicintMicrosecond{get;}publicintNanosecond{get;}}}
Current state of implementation
  •  Proposal logic / implementation
  •  Ref Assembly
  •  Tests
/cc @tarekgh

/cc @ChristopherHaws
Author:deeprobin
Assignees:-
Labels:
area-System.Runtime, new-api-needs-documentation, community-contribution
Milestone:-
@tarekghtarekgh
self-assigned this
Apr 6, 2022
@tarekgh
tarekgh
commented
Apr 6, 2022

Copy link


Copy Markdown

Member
@deeprobin I'll try to take a look at this PR later today and will get back to you.
tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/TimeSpan.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 6, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

@danmoseley
danmoseley
commented
Apr 7, 2022

Copy link


Copy Markdown

Contributor
@deeprobin you might be interested to check that your tests are giving full code coverage of your code:
runtime/docs/workflow/building/libraries/code-coverage.md

Line 53
in
b248950
## Code coverage with System.Private.CoreLib code
@deeprobin
deeprobin
commented
Apr 7, 2022

Copy link


Copy Markdown

ContributorAuthor
@danmoseley

Thanks. I can take a look at that after the other open questions have been clarified here :).
tarekgh
tarekgh
reviewed
Apr 7, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 9, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeOffsetTests.cs
tarekgh
tarekgh
reviewed
Apr 12, 2022
View reviewed changes
Comment threadsrc/libraries/System.Runtime/tests/System/DateTimeTests.cs
@tarekgh
tarekgh
commented
Apr 12, 2022•

edited

Loading

Copy link


Copy Markdown

Member
Could you please add tests for TimeSpan new APIs? Also, when writing these tests please ensure using negative values in addition to positive values.
@tarekgh
tarekgh
commented
Apr 12, 2022

Copy link


Copy Markdown

Member
@deeprobin I left some comments I hope you can address them soon. After that we should be good to go. great work!
@joperezr
joperezr
commented
Apr 12, 2022•

edited

Loading

Copy link


Copy Markdown

Member
/azp run runtime (Rerunning CI in order to fetch changes merged in main which have fixed the Regex test)
@azure-pipelines
azure-pipelinesBot
commented
Apr 12, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@dotnetdotnet
deleted a comment from azure-pipelinesBotApr 12, 2022
danmoseley
danmoseley
reviewed
Apr 13, 2022
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/DateTime.cs
Outdated

@tarekgh
tarekgh
commented
Apr 13, 2022

Copy link


Copy Markdown

Member
@deeprobin will be able to address the remaining feedback soon?
@deeprobin
deeprobin
requested a review
from tarekghApril 13, 2022 18:11
@tarekgh
tarekgh
commented
Apr 13, 2022

Copy link


Copy Markdown

Member
The failing test is unrelated and is tracked by the issue #67878
@tarekgh
tarekgh
merged commit b2ed250
into
dotnet:mainApr 14, 2022
@ChristopherHaws
ChristopherHaws
commented
Apr 14, 2022

Copy link


Copy Markdown

🎉 Thanks for implementing this @deeprobin!
@akoeplingerakoeplinger
mentioned this pull request
Apr 14, 2022
 Merged

@deeprobindeeprobin
mentioned this pull request
Apr 14, 2022
 Merged

radical added a commit
to radical/runtime
that referenced
this pull request
Apr 14, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
e87bda5
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
@radicalradical
mentioned this pull request
Apr 14, 2022
 Merged

radical added a commit
to thaystg/runtime
that referenced
this pull request
Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main'
79c9fde
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
dotnet#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
radical added a commit
that referenced
this pull request
Apr 15, 2022
@radical
[wasm][debugger] Fix tests broken on 'main' (#68054)
895715e
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
thaystg added a commit
that referenced
this pull request
Apr 26, 2022
@thaystg@radical
[wasm][debugger] Debug on firefox (#61776)
a970720
* First compiling and working version just proxying messages.
* almost working, already showing the cs files
* Working on firefox.
* Use internal e not public.
* Debugging on firefox working.
* Working after the merge
* Keep the TcpListener open and use a random port.
* Show null value.
* - Show JS Callstack
- Skip properties
- Get array value from evaluateAsyncJS and not use the preview value.
* Fix compilation
* Infrastructure to run debugger tests.
* fix merge
* run test.
* Skipping tests that are not passing on Firefox.
* Skipping tests that are not passing on Firefox.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Passing 13 steppingtests.
* Failed: 0, Passed: 39, Skipped: 203, Total: 242, Duration: 5 m 6 s - DebuggerTestSuite.dll (net6.0)
* Failed: 0, Passed: 66, Skipped: 195, Total: 261, Duration: 9 m 29 s - DebuggerTestSuite.dll (net6.0)
* Using ConditionalTheory and ConditionalFact implemented by @radical.
* Fixing side effect.
* Implemented conditional breakpoints.
Failed: 0, Passed: 74, Skipped: 189, Total: 263, Duration: 8 m 41 s - DebuggerTestSuite.dll (net6.0)
* Fix special characters and pointers.
Failed: 0, Passed: 116, Skipped: 177, Total: 293
* Fix merge
* Run debugger-tests on firefox using codespace
* Starting firefox correctly not stopping in the breakpoint yet.
* Remove unnecessary change
* Fix pause behavior (now showing correctly, pause on breakpoint, pause while stepping)
Start implementing evaluate expressions, working correctly on VSCode.
* Fix local tests.
* Fix missing )
* Passing 190 tests, evaluate expressions working.
* Remove Task.Delays.
Move some attributes from FirefoxMonoProxy to FirefoxExecutionContext.
* Fix container creation
* Trying to run firefox tests on CI.
* Moving file to the right place.
* Trying to run debugger-tests using firefox on CI.
* fixing path
* Missing url to download firefox on helix.
* On run the tests only on linux.
* Trying to download firefox on helix.
* fix error on helix-wasm.targets.
* trying to fix ci
* trying to install firefox on helix.
* Fixing firefox path
* Fix debugger tests on firefox
* fixing profile path
* Install libdbus-glib-1-2 on docker and on codespace
* Trying to run using firefox on CI
* update docker image
* Adding more messages to see errors on CI
* Trying to make it work on CI
* Real test on CI
* Trying to use the firefox machine only to run firefox tests
Retrying connection to Proxy
Remove extra messages added to help to fix CI
* Fix CI
* Fix CI
* Fix CI.
* Remove unnecessary changes.
* Using machine with sudo installed
* Addressing @lewing comments
* Fix run tests on codespace
Using image with python3
* Use default image to build and new image only to run firefox tests
* Fix unrelated change
* Fix ci
* check python version
* Print python versions
* Using image with PIP installed
* Using image with pip updated
* Remove unrelated changes
Increase time to wait for firefox to be ready
* Trying to fix evaluate tests.
* Fix evaluateoncallframe tests
* Trying to fix evaluation tests.
* trying to fix evaluateoncallframetests
* fiz evaluateoncallframetests
* Trying to kill firefox to avoid errors.
* Trying to fix EvaluateOnCallFrameTests
* Fix CI
* Remove failing test
* Fix misctests
* Fix other build errors.
* Trying to fix CI.
* Fix CI
* Remove unecessary message.
* Update src/tests/BuildWasmApps/Wasm.Debugger.Tests/Wasm.Debugger.Tests.csproj
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Merge error while accept @radical suggestion
* Merge error while accept @radical suggestion
* Update src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
Co-authored-by: Ankit Jain <radical@gmail.com>
* Apply suggestions from code review
Co-authored-by: Ankit Jain <radical@gmail.com>
* Addressing @radical comments
* Abort the tcp connection if the proxy throws an exception
* Refactor a bit
* Use more compile time checks for chrome vs firefox
* fix pipeline
* Make debugger job names unique by including the browser
* fix runtime-wasm pipeline
* fix firefox ci job
* split into more files
* cleanup
* Add support for running chrome, and firefox tests in the same job
* fix yml
* fix build
* fix build
* fix windows build
* Don't delete profile folder nor pkill firefox
* Delete and create a new profile folder for each execution
* fix helix command line
* [wasm][debugger] Fix tests broken on 'main'
This test broke because it was checking for the number of members on
`System.TimeSpan`, and that changed with
#67666 , which added new members
like `TotalNanoseconds`.
The test shouldn't depend on this number anyway, so remove that.
```
Failed DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(line: 137, col: 12, method_name: "MethodWithLocalsForToStringTest", call_other: False, invoke_async: False) [758 ms]
Error Message:
[ts_props] Number of fields don't match, Expected: 12, Actual: 16
Expected: True
Actual: False
Stack Trace:
at DebuggerTests.DebuggerTestBase.CheckProps(JToken actual, Object exp_o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 800
at DebuggerTests.DebuggerTestBase.CompareObjectPropertiesFor(JToken locals, String name, Object o, String label, Int32 num_fields) in /_/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs:line 908
at DebuggerTests.MiscTests.InspectLocalsForToStringDescriptions(Int32 line, Int32 col, String method_name, Boolean call_other, Boolean invoke_async) in /_/src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs:line 559
```
* wip
* big refactor
* chrome runs
* ff runs
* ff runs
* cleanup
* cleanup
* cleanup
* change console verbosity to info, for proxy
* More refactoring
* More refactoring, and fix some issues with connections, and other
cleanup
* some cleanup
* fix file name
* Improve cleanup after tests
* some refactoring, fixing some hangs, faster failures etc
* Fix BrowserCrash test for chrome
* fix up logging
* Improve error handling for the proxy running independently
* fix debugging from vscode
* proxy host: add --log-path for logs
* support canceling for the proxy host too, and distinguish different instances of the proxy
* Fix debugger after refreshing the debugged page.
* Fixing chrome debugging.
* Fix startup to work on chrome and also on firefox.
Co-authored-by: Ankit Jain <radical@gmail.com>
@ghostghost
locked as resolved and limited conversation to collaborators
May 14, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@tarekghtarekghtarekgh approved these changes

+1 more reviewer

@danmoseleydanmoseleydanmoseley left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@tarekghtarekgh

Labels

area-System.DateTimecommunity-contributionIndicates that the PR has been added by a community membernew-api-needs-documentation

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Add Microseconds and Nanoseconds to TimeStamp, DateTime, DateTimeOffset, and TimeOnly


6 participants

@deeprobin@tarekgh@danmoseley@joperezr@ChristopherHaws@jeffhandley