Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
  • May 08 19:58
    AArnott commented #584
  • May 07 20:24
    lifengl edited #847
  • May 07 20:22
    lifengl opened #847
  • May 07 07:44
    Bosch-Eli-Black closed #827
  • May 07 07:44
    Bosch-Eli-Black commented #827
  • May 07 06:05
    sharwell commented #827
  • May 06 16:30
    nbarbettini commented #584
  • May 06 16:28
    nbarbettini commented #584
  • May 06 16:28
    nbarbettini commented #584
  • May 06 16:28
    nbarbettini commented #584
  • May 06 16:27
    nbarbettini commented #584
  • May 05 21:07

    AArnott on main

    Crash the process on unexpected… Merge pull request #846 from AA… (compare)

  • May 05 21:07
    AArnott closed #846
  • May 05 16:02
    AArnott unlabeled #845
  • May 05 16:02

    AArnott on main

    Remove unnecessary warning supp… Merge pull request #845 from AA… (compare)

  • May 05 16:02
    AArnott closed #845
  • May 05 15:52
    AArnott labeled #845
  • May 05 15:48
    AArnott edited #846
  • May 05 15:48
    AArnott edited #846
  • May 05 15:47
    AArnott review_requested #846
Andrew Arnott
@AArnott
As for code coverage, I like the suggestion. I've used the VS code coverage tool but that's in-IDE only.
(well, or in VSTS but we're using appveyor now)
Sam Harwell
@sharwell
I recommend disabling the wiki actually. The suggestion is based on the difficulty of code reviewing proposed changes to the documentation that I experienced a couple years ago. I don't know if things have changed in that regard, but overally it seems just as easy to include docs in the repository and doing that means anyone can contribute.
Andrew Arnott
@AArnott
All good points. The point for the wiki is you don't have to worry about stale docs in some release branch.
But then, docs that are consistent with the version of the software is also a good thing.
Andrew Arnott
@AArnott
@sharwell FWIW, the blog post you refer to is linked at the bottom of our threading rules doc
Regarding code coverage, I suppose integrating code coverage results into PRs requires more work than just the appveyor.yml change. I'll need to research that.
Sam Harwell
@sharwell
@AArnott enabling code coverage in PRs is similar to enabling AppVeyor in PRs
you enable the functionality in your codecov.io account, and then you trigger it by having your AppVeyor build send it data.
Andrew Arnott
@AArnott
I just tried running opencover on my local box. It spat out a bunch of errors because it can't find source to xunit itself. But I don't see any code in your change to filter that out. How did you resolve that?
nm. I found it.
Andrew Arnott
@AArnott
Any idea how I can exclude the test sources from my report? These are the parameters I have so far.
Sam Harwell
@sharwell
Why exclude test sources?
excludebyattribute might cover StaFact. Are your tests in a namespace that makes it obvious?
Andrew Arnott
@AArnott
because I want the overall coverage % to reflect product coverage -- not test coverage.
Sam Harwell
@sharwell
I think for one exclusion I actually set it in codecov.io instead of in the build
so the data is sent to codecov.io but then dropped on their end
Andrew Arnott
@AArnott
I'm having a hard time finding good docs.
For example, what exactly is the pattern matching supported in the -excludeByFile switch
and what is the codecov yml file format?
or rather, which settings are supported
Sam Harwell
@sharwell
There's yml format?
Andrew Arnott
@AArnott
yes
also there seems to be a stability problem. Besides making each build 4 minutes instead of 2 (which I can live with), the latest build is 11+ minutes and growing. I suspect it's actually hung.
Sam Harwell
@sharwell
there's a possibility that opencover somehow gets strong references to objects you think should be collectible
I've seen code coverage tooling change results for GC-related tests
I normally run GC tests separately from coverage
Andrew Arnott
@AArnott
Thanks. That's pretty limited. I was hoping it could take the ExcludeByAttribute switch to the yml as well.
Any idea where the doc is for what this means: -filter:"+[Microsoft.VisualStudio.Threading*]*"?
Andrew Arnott
@AArnott
Sam Harwell
@sharwell
It appears there is a race condition in AsyncAutoResetEvent.Set, where it's possible for a task to be cancelled between when Set calls trySetResultToDefault and when the result is actually set.
Sam Harwell
@sharwell
Ah, I see, you protect in from cancellation in that case by essentially unlinking the cancellation token. Interesting.
Andrew Arnott
@AArnott
it's been a long time since I was in the zone for that code. So I'm glad you are satisfied or it would take me a while of study to figure it out again. :)
Sam Harwell
@sharwell
@AArnott :bug: Microsoft/vs-threading#75
Andrew Arnott
@AArnott
Nice catch.
Sam Harwell
@sharwell
@AArnott I think AsyncQueueTests.NoLockHeldForCancellationContinuation would benefit from an assertion at the end that the queue contains one item.
Andrew Arnott
@AArnott
Thanks. Please feel free to file issues for these suggestions (either an individual issues or one with a checklist of your suggestions) so I don't lose track of them in this stream of messages.
Sam Harwell
@sharwell
NP wasn't sure if you wanted small issues like that but they are easy to file
Andrew Arnott
@AArnott
Well, I normally wouldn't. But you tend to have a good level of insight. And ya, if you expect lots of small ideas, a single issue with a markdown checkbox list would probably be preferable.
Sam Harwell
@sharwell
Normally I would just submit pull requests but I'm mostly reviewing the code in the context of different project so issues it is for now :smile:
Sam Harwell
@sharwell
@AArnott Do you have more information about when .NET inlines continuations that don't have ExecuteSynchronously set explicitly?
Andrew Arnott
@AArnott
@sharwell don't submit PRs at this point. Our CONTRIB doc mentions this. We can't accept external PRs at the moment. However if interest is significant enough (from folks like yourself) we will prioritize getting that changed.
Sam Harwell
@sharwell
I did see that but thanks for the heads-up
Andrew Arnott
@AArnott
I'm not sure. I just know that that flag is a suggestion, but TPL may or may not inline the continuation in spite of its presence or absence.
Sam Harwell
@sharwell
PRs also serve to better demonstrate some of the items I've mentioned even if they can't be accepted directly
Andrew Arnott
@AArnott
It certainly can help encourage inlining. I'm not against the flag. I'm just pointing out that it may not actually win us anything if inlining already tends to happen.
re: PRs: True. But then we might get into this uncomfortable world of "I'm rejecting your PR, but I'm going to copy your code into my own commits". So I suppose if you want to show code, use a gist and keep it gist-y. Don't tempt me to reuse your code. :)
Sam Harwell
@sharwell
I'm like :expressionless: each place I read Till which is only 1 character shorter than Until
Andrew Arnott
@AArnott
Are you referring to test names?