Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

Improve PackingPlanTest #1214

Merged
merged 3 commits into from
Aug 4, 2016

Conversation

billonahill
Copy link
Contributor

Cleanup PackingPlanTest and add assertions for getComponentRamDistribution.

I'm going to add tests for PackingPlan serde to this one and wanted to commit other unrelated cleanup items first.

@@ -25,7 +25,7 @@
import com.twitter.heron.spi.common.Constants;

public class PackingPlanTest {
public static PackingPlan generatePacking(Map<String, List<String>> basePacking) {
private static PackingPlan generatePacking(Map<String, List<String>> basePacking) {
Resource resource =
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need this with the new change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Access isn't needed outside of this class and won't be when I add a serde test, since I was going to add it to this class as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was not clear - I meant the variable "resource"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's still used in the ContainerPlan and the PackingPlan objects below. I just changed to a resource for each instance that was unique, so I could assert on that.

@kramasamy
Copy link
Contributor

👍

@billonahill billonahill merged commit 3d1478c into apache:master Aug 4, 2016
nicknezis pushed a commit that referenced this pull request Sep 14, 2020

* fix checkstyles

* fix checkstyles
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants