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.

Sign up for GitHub

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

Fix PackageFeedSortingTest functional test #6185

Merged
merged 3 commits into from
Jul 23, 2018
Merged

Fix PackageFeedSortingTest functional test #6185

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f937d9d
Fix PackageFeedSortingTest to not depend on 10 unique packages
chenriksson Jul 18, 2018
b1a1adb
Fix
chenriksson Jul 18, 2018
936f829
PR feedback
chenriksson Jul 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 58 additions & 33 deletions tests/NuGetGallery.FunctionalTests/ODataFeeds/V2FeedExtendedTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.IO;
using System.Linq;
using System.Net;
using System.Text;
using System.Threading.Tasks;
using Newtonsoft.Json.Linq;
using NuGetGallery.FunctionalTests.Helpers;
Expand Down Expand Up @@ -193,50 +194,74 @@ public async Task GetUpdates1199RegressionTest()
[Category("P1Tests")]
public async Task PackageFeedSortingTest()
{
var request = WebRequest.Create(UrlHelper.V2FeedRootUrl + @"stats/downloads/last6weeks/");
var response = await request.GetResponseAsync();
var topDownloadsResponse = await GetResponseTextAsync(UrlHelper.V2FeedRootUrl + @"stats/downloads/last6weeks/");

string responseText;
using (var sr = new StreamReader(response.GetResponseStream()))
// Search Gallery v2 feed for the top 10 unique package ids, or as many as exist.
string[] topDownloadsNames = GetPackageNamesFromFeedResponse(topDownloadsResponse);

// Ensure at least 1 package was found.
Assert.NotEmpty(topDownloadsNames);

// Search Gallery statistics for the top downloaded packages.
var statsResponse = await GetResponseTextAsync(UrlHelper.BaseUrl + @"stats/packageversions");

var expectedNames = String.Join(", ", topDownloadsNames);
var last = topDownloadsNames.First();
foreach (var current in topDownloadsNames.Skip(1))
{
responseText = await sr.ReadToEndAsync();
// Check to make sure the top 10 packages are in the same order as the feed.
// We add angle brackets to prevent false failures due to duplicate package names in the page.
var condition = statsResponse.IndexOf(">" + last + "<", StringComparison.Ordinal)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is pretty hacky--we're asserting the order of an HTML table for looking for the index of >{package ID}< and so on.

The real structure is:

<table>
    <tbody>
        <tr>
            <td>{row number}<td>
            <td><a href="/packages/{package ID}">{package ID}</a><td>
            ...
        </tr>
    </tbody>
</table>

I guess this is sufficient for now, but this code would be much more readable if you could find some sort of actual HTML parser for this purpose.

< statsResponse.IndexOf(">" + current + "<", StringComparison.Ordinal);
Assert.True(condition, $"Expected string {last} to come before {current}. Expected list is: {expectedNames}.");

Assert.NotEqual(last, current);
last = current;
}
}

// Grab the top 10 package names in the feed.
string[] packageName = new string[10];
responseText = packageName[0] = responseText.Substring(responseText.IndexOf(@"""PackageId"": """, StringComparison.Ordinal) + 14);
packageName[0] = packageName[0].Substring(0, responseText.IndexOf(@"""", StringComparison.Ordinal));
for (int i = 1; i < 10; i++)
private async Task<string> GetResponseTextAsync(string url)
{
using (var wr = await WebRequest.Create(url).GetResponseAsync())
using (var sr = new StreamReader(wr.GetResponseStream()))
{
responseText = packageName[i] = responseText.Substring(responseText.IndexOf(@"""PackageId"": """, StringComparison.Ordinal) + 14);
packageName[i] = packageName[i].Substring(0, responseText.IndexOf(@"""", StringComparison.Ordinal));
// Sometimes two versions of a single package appear in the top 10. Stripping second and later instances for this test.
for (int j = 0; j < i; j++)
{
if (packageName[j] == packageName[i])
{
packageName[i] = null;
i--;
}
}
return await sr.ReadToEndAsync();
}
}

request = WebRequest.Create(UrlHelper.BaseUrl + @"stats/packageversions");
private string[] GetPackageNamesFromFeedResponse(string feedResponseText)
{
const string PackageIdStartKey = @"""PackageId"": """;
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO the \" syntax is a little easier to read, you've got like a billion "s in here

const string PackageIdEndKey = @"""";

// Get the response.
response = await request.GetResponseAsync();
using (var sr = new StreamReader(response.GetResponseStream()))
{
responseText = await sr.ReadToEndAsync();
}
var results = new List<string>();

Func<string, int> seekStart = s => s.IndexOf(PackageIdStartKey, StringComparison.Ordinal);
Func<string, int> seekEnd = s => s.IndexOf(PackageIdEndKey, StringComparison.Ordinal);

for (int i = 1; i < 10; i++)
do
{
// Check to make sure the top 10 packages are in the same order as the feed.
// We add angle brackets to prevent false failures due to duplicate package names in the page.
var condition = responseText.IndexOf(">" + packageName[i - 1] + "<", StringComparison.Ordinal) < responseText.IndexOf(">" + packageName[i] + "<", StringComparison.Ordinal);
Assert.True(condition, "Expected string " + packageName[i - 1] + " to come before " + packageName[i] + ". Expected list is: " + packageName[0] + ", " + packageName[1] + ", " + packageName[2] + ", " + packageName[3] + ", " + packageName[4] + ", " + packageName[5] + ", " + packageName[6] + ", " + packageName[7] + ", " + packageName[8] + ", " + packageName[9]);
var start = seekStart(feedResponseText);
if (start < 0)
{
break;
}

feedResponseText = feedResponseText.Substring(start + PackageIdStartKey.Length);
Copy link
Contributor

Choose a reason for hiding this comment

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

The response is simple JSON, I think we could easily parse it with JSON.NET and this code would be much simpler.

var packages = JArray.Parse(feedResponseText);
foreach (var package in packages)
{
    yield return ((JObject)package)["PackageId"].Value<string>();
}

Could also model it and use JsonConvert.DeserializeObject, e.g.

private class Last6WeeksStatistics
{
    public IEnumerable<PackageStatistic> PackageStatistics { get; set; }
}

private class PackageStatistic
{
    public string PackageId { get; set; }
}

var end = seekEnd(feedResponseText);
if (end >= 0)
{
var name = feedResponseText.Substring(0, end);
if (!results.Contains(name, StringComparer.Ordinal))
{
results.Add(name);
}
feedResponseText = feedResponseText.Substring(end);
}
}
while (results.Count < 10);

return results.ToArray();
}
}
}