Skip to content
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

Update tests to work with sshkit master / unknown user #67

Merged

Conversation

felixbuenemann
Copy link
Contributor

This accounts for changes in sshkit's local username lookup introduced by capistrano/sshkit#293 and also accounts for the conditional formatting changes introduced by #66.

The latter did not previously cause failed tests, because the username was stubbed in the tests, causing the relevant code to not be exercised.

Now the username is looked up through sshkit, which results in an empty username on sshkit < 1.7.0, so the no-username display is triggered ("on host" vs. "as foo@host").

@mattbrictson
Copy link
Owner

Great! I like that this also simplifies the test setup somewhat. I'll merge once CI is complete.

@felixbuenemann
Copy link
Contributor Author

Yes, I'm no fan of stubs if they can be avoided.

Maybe there should be an explicit tests for the "user@host" vs "host" formatting?

@felixbuenemann
Copy link
Contributor Author

The appveyor tests now run into sshkit's windows incompatibility before the changes to master, so maybe we need to keep the Etc stub for that? The new sshkit version should be out soon, at which point the appveyor tests would work.

@mattbrictson
Copy link
Owner

Yes, you're right. Unfortunately I think the Etc stub needs to stay in there, for Windows SSHKit <= 1.7.1.

This accounts for changes in sshkit's local username lookup introduced
by capistrano/sshkit#293 and also accounts for the conditional
formatting changes introduced by mattbrictson#66.

The latter did not previously cause failed tests, because the username
was stubbed in the tests, causing the relevant code to not be exercised.

Now the username is looked up through sshkit, which results in an empty
username on sshkit < 1.7.0, so the no-username display is triggered.

Unfortunately we need to keep the Etc stub for now to avoid crashing on
Windows with sshkit 1.7.0 and 1.7.1.
@felixbuenemann
Copy link
Contributor Author

OK, I've added back the Etc stub for sshkit <= 1.7.1.

@mattbrictson
Copy link
Owner

Thanks!

mattbrictson added a commit that referenced this pull request Nov 1, 2015
Update tests to work with sshkit master / unknown user
@mattbrictson mattbrictson merged commit a642958 into mattbrictson:master Nov 1, 2015
@felixbuenemann felixbuenemann deleted the fix-tests-for-sshkit-master branch November 1, 2015 23:49
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.

None yet

2 participants