Skip to content

add test for opening and reading from device interface#54673

Merged
adamsitnik merged 7 commits intodotnet:mainfrom
adamsitnik:deviceIds
Jun 30, 2021
Merged

add test for opening and reading from device interface#54673
adamsitnik merged 7 commits intodotnet:mainfrom
adamsitnik:deviceIds

Conversation

@adamsitnik
Copy link
Member

@adamsitnik adamsitnik commented Jun 24, 2021 •
edited
Loading

test for #54143

@adamsitnik adamsitnik added this to the 6.0.0 milestone Jun 24, 2021
@ghost
Copy link

ghost commented Jun 24, 2021

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

Issue Details
Author: adamsitnik
Assignees: -
Labels:

area-System.IO
Milestone: 6.0.0

@adamsitnik
Copy link
Member Author

adamsitnik commented Jun 24, 2021

@stephentoub what is your opinion about having tests like this? On one hand we get a very unique test code coverage, but on the other the amount of sys-calls required to setup is very high.

@stephentoub
Copy link
Member

stephentoub commented Jun 24, 2021

on the other the amount of sys-calls required to setup is very high.

What is the concern with that?

@adamsitnik
Copy link
Member Author

adamsitnik commented Jun 24, 2021

What is the concern with that?

I was afraid that others may perceive it as more complexity than value being added.

@stephentoub
Copy link
Member

stephentoub commented Jun 24, 2021

It can always be made outerloop, which would be good to do anyway if it's mucking with the machine in some way.

@adamsitnik adamsitnik mentioned this pull request Jun 25, 2021
Open
8 tasks
@adamsitnik
Copy link
Member Author

adamsitnik commented Jun 25, 2021

/azp run runtime-libraries-coreclr outerloop-windows

@azure-pipelines
Copy link

azure-pipelines bot commented Jun 25, 2021

Azure Pipelines successfully started running 1 pipeline(s).

@adamsitnik adamsitnik closed this Jun 25, 2021
@adamsitnik adamsitnik reopened this Jun 25, 2021
@adamsitnik
Copy link
Member Author

adamsitnik commented Jun 25, 2021

/azp run runtime-libraries-coreclr outerloop-windows

@azure-pipelines
Copy link

azure-pipelines bot commented Jun 25, 2021

Azure Pipelines successfully started running 1 pipeline(s).

@runfoapp runfoapp bot mentioned this pull request Jun 25, 2021
Closed
jozkee
jozkee approved these changes Jun 25, 2021
View reviewed changes
Copy link
Member

@jozkee jozkee left a comment

Choose a reason for hiding this comment

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

Otherwise lgtm; thanks, Adam.

carlossanlop
carlossanlop approved these changes Jun 25, 2021
View reviewed changes
Copy link
Contributor

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

I only have a question about the line removed from the *.csproj. Otherwise, LGTM.

@adamsitnik
Copy link
Member Author

adamsitnik commented Jun 30, 2021

/azp run runtime-libraries-coreclr outerloop-windows

@azure-pipelines
Copy link

azure-pipelines bot commented Jun 30, 2021

Azure Pipelines successfully started running 1 pipeline(s).

@adamsitnik adamsitnik merged commit 38df267 into dotnet:main Jun 30, 2021
@adamsitnik adamsitnik deleted the deviceIds branch June 30, 2021 14:57
@adamsitnik adamsitnik mentioned this pull request Jul 1, 2021
Closed
@karelz karelz mentioned this pull request Jul 28, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Jul 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@danmoseley danmoseley danmoseley left review comments

@jkotas jkotas jkotas left review comments

@jozkee jozkee jozkee approved these changes

+1 more reviewer

@carlossanlop carlossanlop carlossanlop approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-System.IO

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

6 participants

@adamsitnik @stephentoub @carlossanlop @danmoseley @jkotas @jozkee