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

Increase Spark.Worker.UnitTest test coverage for different Spark versions #638

Merged
merged 7 commits into from
Aug 26, 2020

Conversation

KimKiHyuk
Copy link
Contributor

@KimKiHyuk KimKiHyuk commented Aug 21, 2020

There are a lot of spark versions but only 2.4.0 has been tested.

so I've added test cases, also move test cases to TestData.cs

also moved test cases have been reused in PayloadProcessTests

@KimKiHyuk KimKiHyuk force-pushed the tests/refactor-DaemonWorkerTests branch from d6c4512 to 448c411 Compare August 21, 2020 19:26
@KimKiHyuk KimKiHyuk changed the title Refactor DaemonWorkerTests and split test data Refactor DaemonWorkerTests and make more version test cases Aug 21, 2020
@KimKiHyuk KimKiHyuk changed the title Refactor DaemonWorkerTests and make more version test cases Add testcases for spark version Aug 21, 2020
@imback82
Copy link
Contributor

Looks like tests are failing?

@KimKiHyuk
Copy link
Contributor Author

KimKiHyuk commented Aug 22, 2020

Looks like tests are failing?

it seems to occur outofmemory exception
https://dev.azure.com/dnceng/public/_build/results?buildId=782534&view=logs&j=3dc8fd7e-4368-5a92-293e-d53cefc8c4b3

when test case only 2.4.0, CI was passed.

I think CI test machine memory is not enough to test all spark versions.

@imback82
Copy link
Contributor

Can you check which version is causing the failure?

@KimKiHyuk
Copy link
Contributor Author

Can you check which version is causing the failure?

It was my fault. The problem was a test logic and it has been resolved in cf191ea.

thank you :)

@@ -2,6 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
Copy link
Member

Choose a reason for hiding this comment

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

clean up usings

Comment on lines 17 to 25
public static IEnumerable<object[]> versionTests()
{
yield return new object[] { Versions.V2_3_0 };
yield return new object[] { Versions.V2_3_1 };
yield return new object[] { Versions.V2_3_2 };
yield return new object[] { Versions.V2_3_3 };
yield return new object[] { Versions.V2_4_0 };
yield return new object[] { Versions.V3_0_0 };
}
Copy link
Member

Choose a reason for hiding this comment

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

nit

Suggested change
public static IEnumerable<object[]> versionTests()
{
yield return new object[] { Versions.V2_3_0 };
yield return new object[] { Versions.V2_3_1 };
yield return new object[] { Versions.V2_3_2 };
yield return new object[] { Versions.V2_3_3 };
yield return new object[] { Versions.V2_4_0 };
yield return new object[] { Versions.V3_0_0 };
}
public static IEnumerable<object[]> VersionData() =>
new List<object[]>
{
new object[] { Versions.V2_3_0 },
new object[] { Versions.V2_3_1 },
new object[] { Versions.V2_3_2 },
new object[] { Versions.V2_3_3 },
new object[] { Versions.V2_4_0 },
new object[] { Versions.V3_0_0 }
};

Copy link
Member

@suhsteve suhsteve left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @KimKiHyuk !

Copy link
Contributor

@imback82 imback82 left a comment

Choose a reason for hiding this comment

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

LGTM as well, thanks @KimKiHyuk!

@imback82 imback82 merged commit 5e7ff2d into dotnet:master Aug 26, 2020
@imback82 imback82 changed the title Add testcases for spark version Increase Spark.Worker.UnitTest test coverage for different Spark versions Aug 26, 2020
@imback82 imback82 added the enhancement New feature or request label Aug 26, 2020
@imback82 imback82 added this to the 1.0.0 milestone Aug 26, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants