Functional test for temp keys #3664
Conversation
chenriksson
requested review from
joelverhagen,
skofman1,
scottbommarito and
dtivel
|
|
||
| var json = JObject.Parse(responseText); | ||
| var expiration = json.Value<DateTime>("Expires"); | ||
| Assert.True(DateTime.UtcNow - expiration < TimeSpan.FromMinutes(5)); |
There was a problem hiding this comment.
Assert.True can have a second parameter which is reason. It should be used to express more details about this expression if it fails (the delta perhaps?).
|
|
||
| var json = JObject.Parse(responseText); | ||
| var expiration = json.Value<DateTime>("Expires"); | ||
| Assert.True(DateTime.UtcNow - expiration < TimeSpan.FromMinutes(5)); |
There was a problem hiding this comment.
DateTime.UtcNow should be about 1 day less than expiration right? The resulting TimeSpan from this subtraction will be close to a negative day. Perhaps we could assert that the delta is something close to 24 hours?
| [Category("P1Tests")] | ||
| public async Task VerifyPackageKeySupportsFullAndTempApiKeys() | ||
| { | ||
| var packageId = $"VerifyPackageKeySupportsFullAndTempApiKeys.{DateTime.UtcNow.Ticks}"; |
There was a problem hiding this comment.
nit: use DateTimeOffset whenever possible
There was a problem hiding this comment.
Ticks is used by many other test cases, but can switch if you feel strongly.
There was a problem hiding this comment.
Sorry, that was confusing. I mean DateTimeOffset.Ticks instead of DateTime.Ticks. And DateTimeOffset everywhere else in your new code.
Truth be told you can do most of the same stuff with DateTime and DateTimeOffset, but I think DateTimeOffset should be our default stance since it forces you to think about timezone every time you use it. It may be irrelevant in this case where we just want to make a unique package ID but I think consistency is better so we don't have to think about the quirky differences between the two APIs.
There was a problem hiding this comment.
This is very much a style comment vs. a comment on functionality.
There was a problem hiding this comment.
No worries - switched for my test alone, but maybe I'll set a trend here. :-)
| var verificationKey = await CreateVerificationKey(packageId, packageVersion); | ||
|
|
||
| Assert.Equal(HttpStatusCode.OK, await VerifyPackageKey(verificationKey, packageId, packageVersion)); | ||
| Assert.Equal(HttpStatusCode.Forbidden, await VerifyPackageKey(verificationKey, packageId, packageVersion)); |
There was a problem hiding this comment.
I'd rather see each scenario as an individual test instead of having one mega test cover all scenarios for VerifyPackageKey(...).
| request.Method = "POST"; | ||
| request.ContentLength = 0; | ||
| request.Headers.Add("X-NuGet-ApiKey", EnvironmentSettings.TestAccountApiKey); | ||
| request.Headers.Add("X-NuGet-Client-Version", "NuGetGallery.FunctionalTests"); |
There was a problem hiding this comment.
On the server side do we treat this value as an opaque string or do we try to parse it as a version? I assume it's the former but just checking.
…7.03.27 * tag 'v2017.03.27': (205 commits) Revert UpdateIsLatest optimistic concurrency changes (NuGet#3707) UpdateIsLatest concurrent unlist fix (NuGet#3695) Change telemetry time to use correct format (NuGet#3690) Fix typo of "publically" (NuGet#3636) Fix regression (NuGet#3667) Add credential to Register and RequestPasswordReset audits (NuGet#3666) Functional test for temp keys (NuGet#3664) Telemetry for temp keys (NuGet#3662) Temp keys implementation (NuGet#3563) (NuGet#3646) Extracting code: single type per file (NuGet#3644) Telemetry for package push (NuGet#3649) Upgrade to NuGet.* v4.0.0 dependencies (NuGet#3643) Fix concurrent push test by disabling search hijacking on feed (NuGet#3641) Fixing Package Description truncation (NuGet#3638) Fix Microsoft Account removal (NuGet#3639) Send e-mail when a new API key is created (NuGet#3634) IsLatest Fix: wrong connection string passed to retry context (NuGet#3632) Update WindowsAzure.Storage to 7.0.0 (NuGet#3633) Depend on signed version of Elmah (NuGet#3609) Move AzureEntityList and TableErrorLog to NuGetGallery.Core (NuGet#3607) ...
No description provided.