Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

NegotiateStream Tests on Unix#6139

Merged
kapilash merged 1 commit into
dotnet:dev/negotiatestreamfrom
kapilash:tests/negotiate
Feb 25, 2016
Merged

NegotiateStream Tests on Unix#6139
kapilash merged 1 commit into
dotnet:dev/negotiatestreamfrom
kapilash:tests/negotiate

Conversation

@kapilash
Copy link
Copy Markdown
Contributor

Introducing CI tests for NegotiateStream on Unix.

This PR is introduced as two commits -

  1. where the infrastructure setup (related to installing and configuring kdc) is done as a part of external script (that is run by the jenkins job).

2)where the setup is done via a test fixture.

In order for the tests to run successfully, #5773 needs to be merged.

cc: @stephentoub @vijaykota @shrutigarg



GssBuffer inputToken {.length = UnsignedCast(inputLength), .value = inputBytes};
GssBuffer gssBuffer { .length = 0, .value = nullptr };
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nits: a few formatting things, e.g. extra blank line above, the two initializers not having the same spacing around the fields, etc.

@Priya91
Copy link
Copy Markdown
Contributor

Priya91 commented Feb 16, 2016

With the fixture setup done here, will remove the sample code from #6028

struct PAL_GssBuffer* outBuffer);

/*

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: similar formatting nits, e.g. blank line, function arguments not aligned, etc.

@stephentoub
Copy link
Copy Markdown
Member

Looks like a good start. Thanks!

@vijaykota
Copy link
Copy Markdown
Contributor

This PR depends on #6158 in addition to #5773.


[Fact, OuterLoop]
[PlatformSpecific(PlatformID.Linux | PlatformID.OSX)]
public void NegotiateStream_StreamToStream_KerberosAuthDefaultCredentials_Success()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let us add a DefaultCreds_failure test case where TGT does not exist before calling AuthenticateAsClientAsync

@kapilash
Copy link
Copy Markdown
Contributor Author

With the fixture setup done here, will remove the sample code from #6028

Thanks @Priya91 This fixture was written based on your sample code.

@Priya91
Copy link
Copy Markdown
Contributor

Priya91 commented Feb 18, 2016

@dotnet-bot Test Outerloop Ubuntu

Initiating Outerloop run to test these changes with the merged changes in #6028

@vijaykota vijaykota mentioned this pull request Feb 18, 2016
@Priya91
Copy link
Copy Markdown
Contributor

Priya91 commented Feb 18, 2016

The netci.groovy changes have been pushed to master, and should be used for outerloop ubuntu14.04 runs. Triggering an outerloop run to verify.

@Priya91
Copy link
Copy Markdown
Contributor

Priya91 commented Feb 18, 2016

@dotnet-bot test outerloop Ubuntu14.04

@Priya91
Copy link
Copy Markdown
Contributor

Priya91 commented Feb 18, 2016

The machine setup works, as the Outerloop Process tests that require admin credentials pass in the run. However, while running setup-kdc.sh there is a permission denied error, as the script is not having execute permission.

Assert.False(client.IsAuthenticated, "client is not authenticated before AuthenticateAsClient call");

Task[] auth = new Task[2];
string user = string.Format("{0}@{1}", TestConfiguration.KerberosUser, TestConfiguration.Realm);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: unused var

@vijaykota
Copy link
Copy Markdown
Contributor

@kapilash , few more changes (uninstall etc.) will be needed since the script has been updated - see #6158

@vijaykota vijaykota added * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) and removed Needs extra testing before merge * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) labels Feb 19, 2016
@kapilash kapilash force-pushed the tests/negotiate branch 2 times, most recently from c26cd43 to f079a7c Compare February 23, 2016 15:20
@kapilash kapilash changed the title NegotiateStream Tests on Unix (do not merge) NegotiateStream Tests on Unix Feb 23, 2016
@kapilash
Copy link
Copy Markdown
Contributor Author

I am removing the (do not merge) tag since all the prereqs have landed into the dev branch.

@vijaykota
Copy link
Copy Markdown
Contributor

LGTM. @stephentoub , please see if the latest commit looks good. This will help unblock @shrutigarg who is working on adding NTLM tests

private static int RunSetupScript(string args = null)
{
ProcessStartInfo startInfo = new ProcessStartInfo();
startInfo.UseShellExecute = true;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: Why true? It looks like the command being launched itself invokes bash.

@stephentoub
Copy link
Copy Markdown
Member

A few more nits, but otherwise looks good. You can merge once the few things are addressed. Thanks!

* Tests install kdc on the local machine via kdc-setup.sh (The script will be run as admin)
@kapilash
Copy link
Copy Markdown
Contributor Author

Thanks @stephentoub - I fixed them.
I am also squashing the commits. Some how the CI machine did not pick up the earlier commit.

@Priya91
Copy link
Copy Markdown
Contributor

Priya91 commented Feb 25, 2016

@dotnet-bot Test Outerloop Ubuntu14.04

@kapilash
Copy link
Copy Markdown
Contributor Author

Outer loop tests failed with :

 java.io.IOException: remote file operation failed: /mnt/j/workspace/dotnet_corefx/outerloop_ubuntu14.04_debug_prtest at hudson.remoting.Channel@3f4a6cea:Azure0224034215: hudson.util.IOException2: There are some problems during the conversion into JUnit reports: 

System.Net.Security.Tests did succeed

 19:19:11   
 19:19:11   === TEST EXECUTION SUMMARY ===
  19:19:11      System.Net.Security.Tests  Total: 10, Errors: 0, Failed: 0, Skipped: 0, Time: 1.087s

@vijaykota
Copy link
Copy Markdown
Contributor

System.Net.Security.Tests did succeed

Thanks for the combined effort, @Priya91 , @rahulkotecha and @kapilash

@Priya91 can you extend the changes for Ubuntu to other outerloop OSs?

@vijaykota
Copy link
Copy Markdown
Contributor

@dotnet-bot, Test Innerloop CentOS7.1

@kapilash
Copy link
Copy Markdown
Contributor Author

Failures in CentOS are due to

System.IO.FileNotFoundException : Could not load file or assembly 'System.Diagnostics.TraceSource, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. The system cannot find the file specified.

@kapilash
Copy link
Copy Markdown
Contributor Author

I am merging this to the dev/negotiate branch to unblock ntlm tests. The tests seem to have run successfully and the failures are all unrelated.
Please give me shout, if it is a problem. I'll revert the commit.

kapilash added a commit that referenced this pull request Feb 25, 2016
@kapilash kapilash merged commit b7634b6 into dotnet:dev/negotiatestream Feb 25, 2016
@karelz karelz added this to the 1.0.0-rtm milestone Jan 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants