ci: Run fuzzer task for the master branch only #22730

hebasto · 2021-08-17T15:09:09Z

I think we need to decide whether running the fuzzer CI in any branch other than master is something we want to be doing / maintaining. This seems pretty unsustainable unless we at least make changes in regards to the fuzz inputs being used by the different branches. I'm pretty sure Marco has mentioned this before.

This PR makes CI ignore fuzz tests by forcing RUN_FUZZ_TESTS=false for all cases when it is not the master branch or a PR based on it.

See #22731 as a demo for the 22.x branch.

hebasto · 2021-08-17T18:12:22Z

cc @fanquake @MarcoFalke

jamesob · 2021-08-17T19:50:35Z

ACK cac87ed

Zero-1729 · 2021-08-18T00:36:52Z

Concept ACK

if [ "${CIRRUS_BRANCH}" == "${CIRRUS_DEFAULT_BRANCH}" ] || [ "${CIRRUS_BASE_BRANCH}" == "${CIRRUS_DEFAULT_BRANCH}" ]; then

I think the second condition where it checks if the PR is based on the master branch should also include an additional check to see if the PR affects the fuzzing system, to narrow it down (i.e. like checking the CIRRUS_CHANGE_TITLE to see if the PR's title is prepended with fuzz: for example, or contains the word fuzz elsewhere in the title). What do you think @hebasto?

fanquake · 2021-08-18T02:20:05Z

I think the second condition where it checks if the PR is based on the master branch should also include an additional check to see if the PR affects the fuzzing system, to narrow it down (i.e. like checking the CIRRUS_CHANGE_TITLE to see if the PR's title is prepended with fuzz: for example, or contains the word fuzz elsewhere in the title).

That sounds broken / over-complicated. On the master branch the fuzzers should be running on all commits, regardless of if the change is to the fuzzers themselves, or has a certain commit prefix. All we want to do is exclude branches other than master.

Zero-1729 · 2021-08-18T03:35:40Z

Ah, thanks for the clarification! Seems I misunderstood the OP.

In that case, ignore my comment, the change looks good to go.

Zero-1729

crACK cac87ed

maflcko · 2021-08-18T05:09:33Z

Ideally the fuzzers would run on all branches, it is just that my personal efforts are focussed on the master branch. While the fuzzers should run on older branches, I won't be working on this actively myself.

About the changes: The changes here won't disable the fuzzers when built locally or run in CI locally. If the goal is to disable the fuzz task in our Cirrus CI, it might be better to just do that in the cirrus.yml and not by putting CI-specific symbols (CIRRUS_*) in our CI-agnostic scripts (./ci).

hebasto · 2021-08-18T09:34:31Z

Updated cac87ed -> fa2234f (pr22730.01 -> pr22730.02).

Addressed @MarcoFalke's comment:

If the goal is to disable the fuzz task in our Cirrus CI, it might be better to just do that in the cirrus.yml and not by putting CI-specific symbols (CIRRUS_*) in our CI-agnostic scripts (./ci).

laanwj · 2021-08-18T15:38:01Z

Concept ACK, although in theory it'd make sense to fuzz some things across versions, I agree in practice it's unmaintainable, it trips up the CI for unrelated reasons too often.

.cirrus.yml

maflcko · 2021-08-18T17:24:54Z

cr ACK 5a9e255 no opinion on the concept, also didn't test

hebasto · 2021-08-18T17:25:56Z

... also didn't test

#22731 could be useful.

fanquake

ACK 5a9e255 - didn't test other than to look at #22731.

hebasto · 2021-08-20T07:22:04Z

@fanquake

Backport to 22.0 (and drop all other CI fuzz task fixes)?

fanquake · 2021-08-20T07:23:21Z

Backport to 22.0 (and drop all other CI fuzz task fixes)?

#22629 (comment)

Github-Pull: bitcoin#22730 Rebased-From: 5a9e255

hebasto · 2021-08-20T07:38:44Z

Backported to 22.x in #22629.

5a9e255 ci: Run fuzzer task for the master branch only (Hennadii Stepanov) Pull request description: bitcoin#22629 (comment): > I think we need to decide whether running the fuzzer CI in any branch other than master is something we want to be doing / maintaining. This seems pretty unsustainable unless we at least make changes in regards to the fuzz inputs being used by the different branches. I'm pretty sure Marco has mentioned this before. This PR makes CI ignore fuzz tests by forcing `RUN_FUZZ_TESTS=false` for all cases when it is not the master branch or a PR based on it. See bitcoin#22731 as a demo for the 22.x branch. ACKs for top commit: MarcoFalke: cr ACK 5a9e255 no opinion on the concept, also didn't test fanquake: ACK 5a9e255 - didn't test other than to look at bitcoin#22731. Tree-SHA512: 48f8f02f1814d4f15293a8804b76d544a08784ea7acd930b5c6d4608564d30aa5a608b1a511386ffda6975feec700c1bbeb86a30a75a7e48a1c5b167a227dbdd

32e1424 Fix build with Boost 1.77.0 (Rafael Sadowski) cb34a0a qt: Handle new added plurals in bitcoin_en.ts (Hennadii Stepanov) 068985c doc: Mention the flat directory structure for uploads (Andrew Chow) 27d43e5 guix: Don't include directory name in SHA256SUMS (Andrew Chow) 88fb7e3 test: fix bug in 22686 (S3RK) 63fec7e clientversion: No suffix #if CLIENT_VERSION_IS_RELEASE (Carl Dong) dfaffbe test: Test for ApproximateBestSubset edge case with too little fees (Andrew Chow) e86b023 wallet: Assert that enough was selected to cover the fees (Andrew Chow) ffc81e2 wallet: Use GetSelectionAmount for target value calculations (Andrew Chow) ce77b45 release: Release with separate SHA256SUMS and sig files (Carl Dong) cb491bd guix-verify: Non-zero exit code when anything fails (Carl Dong) 6a611d2 gui: ensure external signer option remains disabled without signers (Andrew Chow) e9b4487 qt: Fix regression in "Encrypt Wallet" menu item (Hennadii Stepanov) 57fce06 consensus/params: simplify ValidDeployment check to avoid gcc warning (Anthony Towns) e9d30fb ci: Run fuzzer task for the master branch only (Hennadii Stepanov) Pull request description: Backported: 1) #22730 1) bitcoin-core/gui#393 1) #22597 1) bitcoin-core/gui#396 1) #22643 1) #22642 1) #22685 1) #22686 1) #22654 1) #22742 1) bitcoin-core/gui#406 1) #22713 ACKs for top commit: laanwj: Code list-of-backported-PRs review ACK 32e1424 Tree-SHA512: f5e2dd1be6cdcd39368eeb5d297b3ff4418d0bf2e70c90e59ab4ba1dbf16f773045d877b4997511de58c3aca75a978dcf043e338bad23951557e2a27ccc845f6

hebasto · 2021-08-26T20:15:51Z

@fanquake @MarcoFalke

Backport to 0.21?

Github-Pull: bitcoin#22730 Rebased-From: 5a9e255

hebasto · 2021-08-27T06:57:31Z

Backported to 0.21 in #22808.

Github-Pull: bitcoin/bitcoin#22730 Rebased-From: 5a9e255e5a324e7aa0b63a9634aa3cfda9a300bd

d9b18c1 Fix build with Boost 1.77.0 (Rafael Sadowski) 2d7f260 ci: Run fuzzer task for the master branch only (Hennadii Stepanov) Pull request description: Backported: 1) #22730 1) #22713 ACKs for top commit: fanquake: ACK d9b18c1 - Checked the backports and tested building this branch with system Boost 1.76.0 and depends Boost 1.77.0. Tree-SHA512: dc3e0a2b3c1e3e80f6570e329a08ebc5103c233c30562f660432891c90bacb4d88d5373e32b9ac34fd143be46b8e63900ce9f52786b04bc799a4d17ba9fd8499

fanquake · 2022-02-21T13:12:57Z

Reverted in #24292.

Github-Pull: bitcoin/bitcoin#22730 Rebased-From: 5a9e255

hebasto mentioned this pull request Aug 17, 2021

[DO NOT MERGE, DEMO] [22.0] ci: Run fuzz tests for the master branch only #22731

Closed

DrahtBot added the Tests label Aug 17, 2021

hebasto mentioned this pull request Aug 17, 2021

[22.x] rc3 backports #22629

Merged

Zero-1729 reviewed Aug 18, 2021

View reviewed changes

hebasto force-pushed the 210817-ci-fuzz branch from cac87ed to fa2234f Compare August 18, 2021 09:29

hebasto force-pushed the 210817-ci-fuzz branch 3 times, most recently from 6828822 to 3a515fe Compare August 18, 2021 15:33

hebasto force-pushed the 210817-ci-fuzz branch from 3a515fe to 2606ac6 Compare August 18, 2021 16:02

maflcko reviewed Aug 18, 2021

View reviewed changes

.cirrus.yml Outdated Show resolved Hide resolved

ci: Run fuzzer task for the master branch only

5a9e255

hebasto force-pushed the 210817-ci-fuzz branch from 2606ac6 to 5a9e255 Compare August 18, 2021 17:19

hebasto changed the title ~~ci: Run fuzz tests for the master branch only~~ ci: Run fuzzer task for the master branch only Aug 18, 2021

fanquake approved these changes Aug 20, 2021

View reviewed changes

fanquake merged commit ff1e633 into bitcoin:master Aug 20, 2021

fanquake added the Needs backport (22.x) label Aug 20, 2021

hebasto deleted the 210817-ci-fuzz branch August 20, 2021 07:29

hebasto added a commit to hebasto/bitcoin that referenced this pull request Aug 20, 2021

ci: Run fuzzer task for the master branch only

e9d30fb

Github-Pull: bitcoin#22730 Rebased-From: 5a9e255

hebasto removed the Needs backport (22.x) label Aug 20, 2021

hebasto added a commit to hebasto/bitcoin that referenced this pull request Aug 27, 2021

ci: Run fuzzer task for the master branch only

2d7f260

Github-Pull: bitcoin#22730 Rebased-From: 5a9e255

hebasto mentioned this pull request Aug 27, 2021

[0.21] 0.21.2rc2 backports #22808

Merged

fujicoin pushed a commit to fujicoin/fujicoin-22.0 that referenced this pull request Aug 27, 2021

ci: Run fuzzer task for the master branch only

2fcb616

Github-Pull: bitcoin/bitcoin#22730 Rebased-From: 5a9e255e5a324e7aa0b63a9634aa3cfda9a300bd

gwillen pushed a commit to gwillen/elements that referenced this pull request Jul 27, 2022

ci: Run fuzzer task for the master branch only

88f7536

Github-Pull: bitcoin/bitcoin#22730 Rebased-From: 5a9e255

gwillen pushed a commit to gwillen/elements that referenced this pull request Aug 1, 2022

ci: Run fuzzer task for the master branch only

8ec2c4f

Github-Pull: bitcoin/bitcoin#22730 Rebased-From: 5a9e255

bitcoin locked and limited conversation to collaborators Feb 21, 2023

ci: Run fuzzer task for the master branch only #22730

ci: Run fuzzer task for the master branch only #22730

Uh oh!

Conversation

hebasto commented Aug 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Aug 17, 2021

Uh oh!

jamesob commented Aug 17, 2021

Uh oh!

Zero-1729 commented Aug 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fanquake commented Aug 18, 2021

Uh oh!

Zero-1729 commented Aug 18, 2021

Uh oh!

Zero-1729 left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko commented Aug 18, 2021

Uh oh!

hebasto commented Aug 18, 2021

Uh oh!

laanwj commented Aug 18, 2021

Uh oh!

Uh oh!

maflcko commented Aug 18, 2021

Uh oh!

hebasto commented Aug 18, 2021

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

hebasto commented Aug 20, 2021

Uh oh!

fanquake commented Aug 20, 2021

Uh oh!

hebasto commented Aug 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Aug 26, 2021

Uh oh!

hebasto commented Aug 27, 2021

Uh oh!

fanquake commented Feb 21, 2022

Uh oh!

Uh oh!

hebasto commented Aug 17, 2021 •

edited

Loading

Zero-1729 commented Aug 18, 2021 •

edited

Loading

hebasto commented Aug 20, 2021 •

edited

Loading