Skip to content

Conversation

@batmat
Copy link
Member

@batmat batmat commented Dec 10, 2018

No JIRA nor changelog, trivial change. Causing recent build flakiness for no obvious reason (this test was added 1+ year ago).

Proposed changelog entries

Submitter checklist

- [ ] JIRA issue is well described

  • Changelog entry appropriate for the audience affected by the change (users or developer, depending on the change). Examples
    * Use the Internal: prefix if the change has no user-visible impact (API, test frameworks, etc.)
    - [ ] Appropriate autotests or explanation to why this change has no tests
    - [ ] For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

Copy link
Contributor

@Wadeck Wadeck left a comment

Choose a reason for hiding this comment

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

Not sure such approach is really good for the long run.

@batmat
Copy link
Member Author

batmat commented Dec 10, 2018

Not sure such approach is really good for the long run.

I tend to agree. I guess I could just delete the test. I wanted to avoid this, but given the perf of the storage, there's probably no good way out. Or at some point introduce a profile for such flakes/things that require some normal environment. (if you check the comment around the test, you'll see that these perfs are absolutely horrible, it's like 3 times slower than an old 5400 RPM hard-disk)....

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Please just @Ignore this test so we are not bothered with it in CI.

@batmat batmat changed the title Bump timeout to accomodate CI crazily low and varying storage performance @Ignore AtomicFileWriterPerfTest Dec 11, 2018
@batmat
Copy link
Member Author

batmat commented Dec 11, 2018

Right, so given @Wadeck and @jglick feedback, I've finally gone the @Ignore path, though I generally prefer to just delete tests in such case, but well. This test could be useful in the future for testing if someone changes AtomicFileWriter or so I suppose.

@batmat batmat force-pushed the bump-timeout-AtomicFileWriterPerfTest branch from c103019 to b9c9b2b Compare December 11, 2018 08:53
@batmat
Copy link
Member Author

batmat commented Dec 11, 2018

Given the implicit approval of @jglick (because he added that @Ignore too himself in #3799), and approvals from Ramon and Wadeck, I will likely merge this later today once the PR build is successful and if nobody objects in the meantime.

* Was analyzed in https://github.com/jenkinsci/jenkins/pull/3788, to summarize the performances in ci.jenkins.io are
* wildly varying, to say the least, between 2.5 seconds, and 1 min 9 seconds!
*
* So bumping this here to 70 seconds, so we keep being able to catch huge regressions if someone touches the Jenkins
Copy link
Member

Choose a reason for hiding this comment

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

obsolete comment

@batmat
Copy link
Member Author

batmat commented Dec 12, 2018

Superseded by #3799

@batmat batmat closed this Dec 12, 2018
@batmat batmat deleted the bump-timeout-AtomicFileWriterPerfTest branch December 12, 2018 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants