Skip to content

Docker#666 keyscan#731 posixcompatnewunittests #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

bagajjal
Copy link

  1. docker ssh issue
    docker-machine doesn't work with this version of OpenSSH Win32-OpenSSH#666
    a) fdopen changes to accept the /dev/null device
    b) fix the select (using same fdset as readfdset, exceptfdset) issue with the unix opensssh code.

  2. changed keyscan pester test to refer to localhost (127.0.0.1) instead of GitHub.com
    Enable keyutility pester testcases and change ssh-keyscan to use localhost Win32-OpenSSH#731

  3. Fix the ASSERT_HANDLE issue..
    ASSERT_HANDLE should fail if handle is either NULL or INVALID_HANDLE.

  4. Added new testcases for the null device.

}

/* if opening null device, point to Windows equivalent */
if (0 == strncmp(path, NULL_DEVICE, strlen(NULL_DEVICE)))

Choose a reason for hiding this comment

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

should it be strlen(NULL_DEVICE)+1 ?

ASSERT_PTR_NE(handle, INVALID_HANDLE_VALUE); \
ASSERT_PTR_NE(handle, 0); \
retValue = ((handle != INVALID_HANDLE_VALUE) && (handle != NULL)) ? 0 : -1; \
ASSERT_INT_EQ(retValue, 0); \

Choose a reason for hiding this comment

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

reverting this.?
If ASSERT_INT_EQ(retValue, 0) fails, you don't know upfront (from test execution output) whether its due to handle is invalid_handle or NULL.
But with 2 explicit asserts, you know if handle was null or invalid_handle.

@manojampalam manojampalam merged commit 4879602 into latestw_all May 26, 2017
@bagajjal bagajjal deleted the docker#666_keyscan#731_posixcompatnewunittests branch May 27, 2017 00:09
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.

3 participants