8000 SI-6412 remove flaky test by SethTisue · Pull Request #5480 · scala/scala · GitHub
[go: up one dir, main page]
Skip to content

SI-6412 remove flaky test #5480

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
Merged
merged 1 commit into from
Nov 23, 2016
Merged

SI-6412 remove flaky test #5480

merged 1 commit into from
Nov 23, 2016

Conversation

SethTisue
Copy link
Member

I have repeatedly seen this fail CI runs, including recently

as the comment in the test itself says: "I'm not sure this is a great
way to test for memory leaks, since we're also testing how good
the JVM's GC is, and this is not easily reproduced between
machines/over time"

review by @adriaanm

@SethTisue
SI-6412 remove flaky test
a03e7a0 
I have repeatedly seen this fail CI runs, including recently

as the comment in the test itself says: "I'm not sure this is a great
way to test for memory leaks, since we're also testing how good
the JVM's GC is, and this is not easily reproduced between
machines/over time"
@scala-jenkins scala-jenkins added this to the 2.12.1 milestone Oct 26, 2016
@SethTisue
Copy link
Member Author

note, this leaves just two tests that still use partest.MemoryTest:

test/files/run/reflection-mem-glbs.scala
test/files/run/reflection-mem-tags.scala

bit tempted to just toss all three of them. what do you think?

an alternative would be to try to make MemoryTest more robust. maybe sprinkle some additional System.gc()s and Thread.sleeps in there, who knows :-\

@dragos
Copy link
Contributor
dragos commented Oct 28, 2016

I think the MemoryTest class should calculate a "slope" for memory usage and test against that. This one in scala-ide was remarkably stable and at the same time caught some regressions.

@SethTisue
Copy link
Member Author

/rebuild

@SethTisue
Copy link
Member Author

Iulian, I have logged your suggestion as https://issues.scala-lang.org/browse/SI-10076

@SethTisue SethTisue merged commit d5f21af into scala:2.12.x Nov 23, 2016
@SethTisue SethTisue deleted the remove-reflection-mem-typecheck-test branch November 23, 2016 17:48
@SethTisue SethTisue mentioned this pull request Jan 10, 2017
Merged
@scabug scabug mentioned this pull request Apr 7, 2017
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szeiger szeiger szeiger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.12.1
Development

Successfully merging this pull request may close these issues.
4 participants
@SethTisue @dragos @szeiger @scala-jenkins
0