Skip to content

[SPARK-26082][MESOS][FOLLOWUP] Add UT on fetcher cache option on MesosClusterScheduler #23744

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

Jump to bottom
Closed
HeartSaVioR wants to merge 1 commit into from
Closed

[SPARK-26082][MESOS][FOLLOWUP] Add UT on fetcher cache option on MesosClusterScheduler #23744

HeartSaVioR wants to merge 1 commit into from

Conversation

@HeartSaVioR
Copy link
Contributor

@HeartSaVioR HeartSaVioR commented Feb 7, 2019

What changes were proposed in this pull request?

This patch adds UT on testing SPARK-26082 to avoid regression. While #23743 reduces the possibility to make a similar mistake, the needed lines of code for adding tests are not that huge, so I guess it might be worth to add them.

How was this patch tested?

Newly added UTs. Test "supports setting fetcher cache" fails when #23743 is not applied and succeeds when #23743 is applied.

@HeartSaVioR
Copy link
Contributor Author

HeartSaVioR commented Feb 7, 2019

cc. @dongjoon-hyun

@HeartSaVioR HeartSaVioR changed the title [SPARK-26082][FOLLOWUP] Add UT on fetcher cache option on MesosClusterScheduler [SPARK-26082][MESOS][FOLLOWUP] Add UT on fetcher cache option on MesosClusterScheduler Feb 7, 2019
@SparkQA
Copy link

SparkQA commented Feb 7, 2019

Test build #102079 has finished for PR 23744 at commit 8415f35.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

dongjoon-hyun
dongjoon-hyun approved these changes Feb 7, 2019
View reviewed changes
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment •
edited
Loading

Choose a reason for hiding this comment

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

+1, LGTM. Thank you so much, @HeartSaVioR !
Merged to master/2.4/2.3.

dongjoon-hyun pushed a commit that referenced this pull request Feb 7, 2019
@HeartSaVioR @dongjoon-hyun
[SPARK-26082][MESOS][FOLLOWUP] Add UT on fetcher cache option on Meso…
e645743 
…sClusterScheduler

## What changes were proposed in this pull request?

This patch adds UT on testing SPARK-26082 to avoid regression. While #23743 reduces the possibility to make a similar mistake, the needed lines of code for adding tests are not that huge, so I guess it might be worth to add them.

## How was this patch tested?

Newly added UTs. Test "supports setting fetcher cache" fails when #23743 is not applied and succeeds when #23743 is applied.

Closes #23744 from HeartSaVioR/SPARK-26082-add-unit-test.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit b4e1d14)
Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun pushed a commit that referenced this pull request Feb 7, 2019
@HeartSaVioR @dongjoon-hyun
[SPARK-26082][MESOS][FOLLOWUP] Add UT on fetcher cache option on Meso…
3abf45d 
…sClusterScheduler

## What changes were proposed in this pull request?

This patch adds UT on testing SPARK-26082 to avoid regression. While #23743 reduces the possibility to make a similar mistake, the needed lines of code for adding tests are not that huge, so I guess it might be worth to add them.

## How was this patch tested?

Newly added UTs. Test "supports setting fetcher cache" fails when #23743 is not applied and succeeds when #23743 is applied.

Closes #23744 from HeartSaVioR/SPARK-26082-add-unit-test.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit b4e1d14)
Signed-off-by: Dongjoon Hyun <[email protected]>
@HeartSaVioR HeartSaVioR deleted the SPARK-26082-add-unit-test branch February 7, 2019 20:33
@HeartSaVioR
Copy link
Contributor Author

HeartSaVioR commented Feb 7, 2019

Thanks for reviewing and merging, @dongjoon-hyun !

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Feb 9, 2019

My bad. Unfortunately, this breaks branch-2.4/2.3. I'll revert this.

[error] /home/jenkins/workspace/spark-branch-2.4-test-sbt-hadoop-2.7/resource-managers/mesos/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterSchedulerSuite.scala:278: missing parameter type for expanded function ((x$7) => x$7.getCache)
[error]     assert(uris.stream().allMatch(_.getCache))
[error]                                   ^
[error] /home/jenkins/workspace/spark-branch-2.4-test-sbt-hadoop-2.7/resource-managers/mesos/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterSchedulerSuite.scala:302: missing parameter type for expanded function ((x$8) => x$8.getCache.unary_$bang)
[error]     assert(uris.stream().allMatch(!_.getCache))
[error]                                    ^

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Feb 9, 2019 •
edited
Loading

@HeartSaVioR . Could you make separate two PRs for branch-2.4 and branch-2.3?

@HeartSaVioR
Copy link
Contributor Author

HeartSaVioR commented Feb 9, 2019

@dongjoon-hyun No problem. I'll submit two PRs soon.

@HeartSaVioR HeartSaVioR mentioned this pull request Feb 9, 2019
Closed
@HeartSaVioR
Copy link
Contributor Author

HeartSaVioR commented Feb 9, 2019

@dongjoon-hyun
Looks like we don't need to let master and branch-2.4/branch-2.3 being diverged.

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Feb 9, 2019

Yep. For scala-2.11, we need to fix master branch, too.

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Feb 9, 2019

Thank you for making all the fast follow-ups, @HeartSaVioR .

dongjoon-hyun pushed a commit that referenced this pull request Feb 9, 2019
@HeartSaVioR @dongjoon-hyun
[SPARK-26082][MESOS][FOLLOWUP] Fix Scala-2.11 build
b8d6669 
## What changes were proposed in this pull request?

#23744 added a UT to prevent a future regression. However, it breaks Scala-2.11 build. This fixes that.

## How was this patch tested?

Manual test with Scala-2.11 profile.

Closes #23755 from HeartSaVioR/SPARK-26082-FOLLOW-UP-V2.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
@HeartSaVioR
[SPARK-26082][MESOS][FOLLOWUP] Add UT on fetcher cache option on Meso…
175366a 
…sClusterScheduler

## What changes were proposed in this pull request?

This patch adds UT on testing SPARK-26082 to avoid regression. While apache#23743 reduces the possibility to make a similar mistake, the needed lines of code for adding tests are not that huge, so I guess it might be worth to add them.

## How was this patch tested?

Newly added UTs. Test "supports setting fetcher cache" fails when apache#23743 is not applied and succeeds when apache#23743 is applied.

Closes apache#23744 from HeartSaVioR/SPARK-26082-add-unit-test.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
@HeartSaVioR
[SPARK-26082][MESOS][FOLLOWUP] Fix Scala-2.11 build
9c28850 
## What changes were proposed in this pull request?

apache#23744 added a UT to prevent a future regression. However, it breaks Scala-2.11 build. This fixes that.

## How was this patch tested?

Manual test with Scala-2.11 profile.

Closes apache#23755 from HeartSaVioR/SPARK-26082-FOLLOW-UP-V2.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Jul 23, 2019
@HeartSaVioR @kai-chi
[SPARK-26082][MESOS][FOLLOWUP] Add UT on fetcher cache option on Meso…
6d87af4 
…sClusterScheduler

## What changes were proposed in this pull request?

This patch adds UT on testing SPARK-26082 to avoid regression. While apache#23743 reduces the possibility to make a similar mistake, the needed lines of code for adding tests are not that huge, so I guess it might be worth to add them.

## How was this patch tested?

Newly added UTs. Test "supports setting fetcher cache" fails when apache#23743 is not applied and succeeds when apache#23743 is applied.

Closes apache#23744 from HeartSaVioR/SPARK-26082-add-unit-test.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit b4e1d14)
Signed-off-by: Dongjoon Hyun <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Jul 25, 2019
@HeartSaVioR @kai-chi
[SPARK-26082][MESOS][FOLLOWUP] Add UT on fetcher cache option on Meso…
daf6745 
…sClusterScheduler

## What changes were proposed in this pull request?

This patch adds UT on testing SPARK-26082 to avoid regression. While apache#23743 reduces the possibility to make a similar mistake, the needed lines of code for adding tests are not that huge, so I guess it might be worth to add them.

## How was this patch tested?

Newly added UTs. Test "supports setting fetcher cache" fails when apache#23743 is not applied and succeeds when apache#23743 is applied.

Closes apache#23744 from HeartSaVioR/SPARK-26082-add-unit-test.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit b4e1d14)
Signed-off-by: Dongjoon Hyun <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Aug 1, 2019
@HeartSaVioR @kai-chi
[SPARK-26082][MESOS][FOLLOWUP] Add UT on fetcher cache option on Meso…
5767598 
…sClusterScheduler

## What changes were proposed in this pull request?

This patch adds UT on testing SPARK-26082 to avoid regression. While apache#23743 reduces the possibility to make a similar mistake, the needed lines of code for adding tests are not that huge, so I guess it might be worth to add them.

## How was this patch tested?

Newly added UTs. Test "supports setting fetcher cache" fails when apache#23743 is not applied and succeeds when apache#23743 is applied.

Closes apache#23744 from HeartSaVioR/SPARK-26082-add-unit-test.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit b4e1d14)
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dongjoon-hyun dongjoon-hyun dongjoon-hyun approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@HeartSaVioR @SparkQA @dongjoon-hyun