Skip to content

Conversation

@mandel-macaque
Copy link
Contributor

No description provided.

connection.SetStateChangeHandler (ConnectionStateHandler);
connection.SetQueue (DispatchQueue.MainQueue);
connection.Start ();
connectedEvent.WaitOne (500);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add an Assert that the WaitOne succeeded (otherwise random timeouts here will be hard to diagnose).

i.Dispose ();
interfaces = new List<NWInterface> ();
path.EnumerateInterfaces (EnumerateInterfacesHandler);
Assert.AreNotEqual (0, interfaces.Count, "We should have at least on interface.");
Copy link
Member

Choose a reason for hiding this comment

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

one typo.

Copy link
Contributor

Choose a reason for hiding this comment

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

Assert.That (interfaces.Count, Is.GreaterThan (0), "interfaces.Count"); explains it better

Copy link
Contributor

@spouliot spouliot left a comment

Choose a reason for hiding this comment

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

I'm curious about the all false properties before approving :)


void ConnectionStateHandler (NWConnectionState state, NWError error)
{
var errno = (SslStatus)(error != null ? error.ErrorCode : 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

^ where is errno used ?

[Test]
public void HasDnsPropertyTest ()
{
Assert.False (path.HasDns);
Copy link
Contributor

Choose a reason for hiding this comment

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

why are those all false ?

i.Dispose ();
interfaces = new List<NWInterface> ();
path.EnumerateInterfaces (EnumerateInterfacesHandler);
Assert.AreNotEqual (0, interfaces.Count, "We should have at least on interface.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Assert.That (interfaces.Count, Is.GreaterThan (0), "interfaces.Count"); explains it better

if (path != null)
path.Dispose ();
foreach (var i in interfaces)
i.Dispose ();
Copy link
Contributor

Choose a reason for hiding this comment

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

you could call Dispose on connectedEvent

@monojenkins
Copy link
Collaborator

Build success
Build succeeded
API Diff (from stable)
API Diff (from PR only) (no change)
Generator Diff (only version changes)
Test run succeeded

1. Address comments of conn issues.
2. Use a unique conn for all unit tests, is expensive and we do not
care in this class.
3. Modify the teardown to clean interfaces etc.. in the cancel event
from the connection.
4. Move the cb execution to a diff queue to avoid been blocked.
5. Ensure tests do work on device (sim has diff results).
@spouliot spouliot added this to the xcode10 milestone Sep 11, 2018
@monojenkins
Copy link
Collaborator

Build success
Build succeeded
API Diff (from stable)
API Diff (from PR only) (no change)
Generator Diff (only version changes)
Test run succeeded

@mandel-macaque mandel-macaque merged commit e43ca08 into dotnet:xcode10 Sep 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants