jcstress
jcstress copied to clipboard
openjdk
7903754: jcstress: Implement fail-on-error run option
Initial PoC
It currently show how to set up argument, and how the framework will be terminated. Feedback welcomed. Should be finished soon
Progress
- [x] Change must not contain extraneous whitespace
- [ ] Change must be properly reviewed (1 review required, with at least 1 Committer)
Issue
- CODETOOLS-7903754: jcstress: Implement fail-on-error run option (Bug - P4)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jcstress.git pull/157/head:pull/157
$ git checkout pull/157
Update a local copy of the PR:
$ git checkout pull/157
$ git pull https://git.openjdk.org/jcstress.git pull/157/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 157
View PR using the GUI difftool:
$ git pr show -t 157
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jcstress/pull/157.diff
Using Webrev
:wave: Welcome back jvanek! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
![bridgekeeper[bot] avatar](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.
The PR is now moreover done. I will run it through even ore testing through.
Webrevs
- 06: Full (389dd675)
- 05: Full - Incremental (70ee4a84)
- 04: Full - Incremental (a2721637)
- 03: Full - Incremental (ddc606b9)
- 02: Full - Incremental (eeaa08e0)
- 01: Full - Incremental (fae6ff11)
- 00: Full (1dcbd95a)
@judovana This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@judovana This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@judovana This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
Although I understand that your main point is to get rid of complexity, but just one thought jcstress and jmh are not comparable suites. - -foe would be serving as shortcut to FAIL_FAST_VARIANTS set to 1, but the way the jcstress works, that the issue may be occurring "just sometimes" moreover enforces some higher tolerance then just " fail on first error". Wdyt?
Also note that most of the complexity will stay, for the sake of report generation at the end.
~~One more call before redoing it - I really think the ratio will be heavily missed in future QA runs. jcstress is not jmh. When jmh fails, it is usually deterministic and stable. With jcstress it is not the case.~~ You want to have failures. The reason for this is to not waste cycles on machines, where it would lead to hundred, or even hundred of thousands ..well all tests failures. We have it on some 10% so we usually have all results, including failures, but if we get borked hw or jvm, it will fail in hour, and not in two days. Similarly, if the VM is broken jsut a bit, there are again full results, with all failures. That give us excelnt compromise. Simple fail on error will heavily downgrade the usability and usage of jcstress:(
~~As i suggested - I can introduce -foe, which will default to --failFAST=1, or I can make the number optional, and it will default eg to those moreover verified 10%. I can remove the %%, as it as proved itself, but at the end was not used by us.~~
~~Similarly I can remove the double failFAST/failFast. The failFast have advantage that you always have a set finished, but is missing fine-tuning which failFAST have. We are using failFast.~~
~~wdyt?~~
Hi! Simplifed as you wished, and as led from my usage in last half a year. So removed everything confusing, kept just one simple -foe which defautls to die after first error. However I kept in the property, which allows user to set custom treshold, both relative or absolute. Please, allow this to exists. I had it set everywhere in our infra, and it helps a lot to distinguish completely borked machine, from just a bit broken jdk.
@judovana Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.
@judovana Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.
@judovana This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@judovana This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@judovana This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@judovana This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@judovana This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!