Improvement in naming changesets #15943

AnieeG · 2025-01-15T22:33:37Z

It was getting difficult to differentiate between Changeset related methods and other public methods in ccip/changeset. Suffixed all Changeset methods with ..Changeset
All test methods are renamed with TestConfig in name to denote those are explicitly for Test.
OCRParameter derivation is made easier with options to override over the default values easily.

…develop

github-actions · 2025-01-15T22:46:02Z

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

AnieeG · 2025-01-16T17:58:51Z

deployment/ccip/changeset/accept_ownership_test.go

 		},
 	})
 	require.NoError(t, err)

-	assertTimelockOwnership(t, e, allChains, state)
-}
-


Moved these to test_assertions

AnieeG · 2025-01-16T18:01:49Z

deployment/ccip/changeset/deployer_group.go

@@ -102,7 +102,7 @@ func (d *DeployerGroup) getDeployer(chain uint64) (*bind.TransactOpts, error) {
 	return sim, nil
 }

-func (d *DeployerGroup) enact(deploymentDescription string) (deployment.ChangesetOutput, error) {
+func (d *DeployerGroup) Enact(deploymentDescription string) (deployment.ChangesetOutput, error) {


had to export these for _test package import

makramkd · 2025-01-16T18:52:35Z

deployment/ccip/changeset/cs_ccip_home.go

+				PriceFeedChainSelector:             ccipocr3.ChainSelector(feedChainSel),
+				NewMsgScanBatchSize:                merklemulti.MaxNumberTreeLeaves,
+				MaxReportTransmissionCheckAttempts: 5,
+				RMNEnabled:                         os.Getenv("ENABLE_RMN") == "true", // only enabled in manual test


This is pretty strange, we shouldn't be accessing os.Getenv in these funcs IMO.

This is actually done for test.let me handle it in a better way in subsequent ticket

makramkd · 2025-01-16T18:55:04Z

deployment/ccip/changeset/deployer_group.go

-func buildTimelockPerChain(e deployment.Environment, state CCIPOnChainState) map[uint64]*proposalutils.TimelockExecutionContracts {
+func BuildTimelockPerChain(e deployment.Environment, state CCIPOnChainState) map[uint64]*proposalutils.TimelockExecutionContracts {


Do these need to be exported? Don't see it being used outside the changeset pkg.

it's used in test -

chainlink/deployment/ccip/changeset/cs_rmn_curse_uncurse_test.go

Line 213 in 8411f36

timelocksPerChain := changeset.BuildTimelockPerChain(e.Env, state)

makramkd · 2025-01-16T18:58:09Z

deployment/ccip/changeset/deployer_group.go

-func buildTimelockAddressPerChain(e deployment.Environment, state CCIPOnChainState) map[uint64]common.Address {
-	timelocksPerChain := buildTimelockPerChain(e, state)
+func BuildTimelockAddressPerChain(e deployment.Environment, state CCIPOnChainState) map[uint64]common.Address {


makramkd · 2025-01-16T18:58:15Z

deployment/ccip/changeset/deployer_group.go

-func buildProposerPerChain(e deployment.Environment, state CCIPOnChainState) map[uint64]*gethwrappers.ManyChainMultiSig {
+func BuildProposerPerChain(e deployment.Environment, state CCIPOnChainState) map[uint64]*gethwrappers.ManyChainMultiSig {


Double ditto

All these are used under _test package

makramkd · 2025-01-16T18:58:30Z

deployment/ccip/changeset/test_usdc_helpers.go

Intentional delete right?

makramkd · 2025-01-16T18:58:57Z

deployment/ccip/changeset/testhelpers/test_assertions.go

@@ -19,6 +19,8 @@ import (
 	commonutils "github.com/smartcontractkit/chainlink-common/pkg/utils"
 	"github.com/smartcontractkit/chainlink-common/pkg/utils/tests"

+	"github.com/smartcontractkit/chainlink/deployment/ccip/changeset"
+	changeset2 "github.com/smartcontractkit/chainlink/deployment/common/changeset"


Rename import to changesetcommon or cscommon or whatever

Suggested change

changeset2 "github.com/smartcontractkit/chainlink/deployment/common/changeset"

cscommon "github.com/smartcontractkit/chainlink/deployment/common/changeset"

makramkd · 2025-01-16T18:59:57Z

deployment/ccip/changeset/testhelpers/test_usdc_helpers.go

Nvm seems transferred here

makramkd · 2025-01-16T19:00:36Z

deployment/ccip/changeset/token_info.go

@@ -20,9 +25,26 @@ const (
 	LinkDecimals             = 18
 	WethDecimals             = 18
 	UsdcDecimals             = 6
+	// MockLinkAggregatorDescription This is the description of the MockV3Aggregator.sol contract


Suggested change

// MockLinkAggregatorDescription This is the description of the MockV3Aggregator.sol contract

// MockLinkAggregatorDescription is the description of the MockV3Aggregator.sol contract

makramkd · 2025-01-16T19:01:07Z

deployment/ccip/changeset/token_info.go

+	// MockLinkAggregatorDescription This is the description of the MockV3Aggregator.sol contract
+	// https://github.com/smartcontractkit/chainlink/blob/a348b98e90527520049c580000a86fb8ceff7fa7/contracts/src/v0.8/tests/MockV3Aggregator.sol#L76-L76
+	MockLinkAggregatorDescription = "v0.8/tests/MockV3Aggregator.sol"
+	// MockWETHAggregatorDescription WETH use description from MockETHUSDAggregator.sol


This comment is unintelligible, maybe

Suggested change

// MockWETHAggregatorDescription WETH use description from MockETHUSDAggregator.sol

// MockWETHAggregatorDescription is the description from MockETHUSDAggregator.sol

makramkd · 2025-01-16T19:02:28Z

deployment/ccip/changeset/view_test.go

-	tenv, _ := NewMemoryEnvironment(t, WithChains(3))
-	_, err := ViewCCIP(tenv.Env)
+	tenv, _ := testhelpers.NewMemoryEnvironment(t, testhelpers.WithNumOfChains(3))
+	_, err := changeset.ViewCCIP(tenv.Env)


Do we have a ticket for better test coverage on the views?

All views have individual tests

Actually no some were missing, created a ticket

cl-sonarqube-production · 2025-01-16T19:20:53Z

Quality Gate passed

Issues
3 New issues
2 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.3% Duplication on New Code

See analysis details on SonarQube

github-actions · 2025-01-16T19:53:53Z

Flakeguard Summary

Ran new or updated tests between develop and 85fa2e2 (fixes-for-staging).

View Flaky Detector Details | Compare Changes

Found Flaky Tests ❌

Name	Pass Ratio	Panicked?	Timed Out?	Race?	Runs	Successes	Failures	Skips	Package	Package Panicked?	Avg Duration	Code Owners
TestRMNCurseIdempotent	0.00%	true	true	false	3	0	3	0	github.com/smartcontractkit/chainlink/deployment/ccip/changeset	true	0s	@smartcontractkit/ccip, @smartcontractkit/core, @smartcontractkit/deployment-automation

Artifacts

For detailed logs of the failed tests, please refer to the artifact failed-test-results-with-logs.json.

AnieeG added 19 commits December 6, 2024 16:47

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

ded3f4c

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

10314c8

…develop

remove deployCCIPContracts

5f14bf0

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

75c676f

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

b31ec5a

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

5f6d2d5

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

65616ff

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

6dd9c4a

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

813cc6e

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

a299e7c

…develop

deprecate existing add lane

1837b21

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

bda53fc

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

e171316

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

52422ba

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

56b0ff1

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

e261d27

…develop

Merge branch 'develop' of github.com:smartcontractkit/chainlink into …

35d936e

…develop

making OCR input more user friendly

59e995c

rename changesets

5e37b44

AnieeG added 2 commits January 15, 2025 15:01

fix lint errors

1da2ad1

fix

7bc1ac4

AnieeG marked this pull request as ready for review January 15, 2025 23:24

AnieeG requested review from a team as code owners January 15, 2025 23:24

AnieeG requested review from winder and asoliman92 January 15, 2025 23:24

AnieeG force-pushed the fixes-for-staging branch from 8f2aa3f to 5f0f701 Compare January 16, 2025 17:05

AnieeG added 3 commits January 16, 2025 09:13

Merge branch 'backup-fixes-for-staging' into fixes-for-staging

a01635c

create new package for test

d8a707a

another move

6d5a3da

AnieeG commented Jan 16, 2025

View reviewed changes

AnieeG added 4 commits January 16, 2025 10:05

more fixes

9f7ae90

fix lint

7d461eb

fix file path

22f5df0

fix file path

533c7ed

AnieeG requested review from a team as code owners January 16, 2025 18:34

go mod tidy

7c5af73

AnieeG requested a review from a team as a code owner January 16, 2025 18:48

AnieeG requested a review from Atrax1 January 16, 2025 18:48

merge with develop

8411f36

makramkd reviewed Jan 16, 2025

View reviewed changes

review comments

85fa2e2

makramkd approved these changes Jan 16, 2025

View reviewed changes

carte7000 approved these changes Jan 16, 2025

View reviewed changes

AnieeG added this pull request to the merge queue Jan 16, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 16, 2025

AnieeG added this pull request to the merge queue Jan 16, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 16, 2025

AnieeG added this pull request to the merge queue Jan 16, 2025

Merged via the queue into develop with commit ac3a55f Jan 16, 2025
195 of 196 checks passed

AnieeG deleted the fixes-for-staging branch January 16, 2025 20:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvement in naming changesets #15943

Improvement in naming changesets #15943

AnieeG commented Jan 15, 2025 •

edited

Loading

github-actions bot commented Jan 15, 2025 •

edited

Loading

AnieeG Jan 16, 2025

AnieeG Jan 16, 2025

makramkd Jan 16, 2025

AnieeG Jan 16, 2025

makramkd Jan 16, 2025

AnieeG Jan 16, 2025

makramkd Jan 16, 2025

makramkd Jan 16, 2025

AnieeG Jan 16, 2025

makramkd Jan 16, 2025

makramkd Jan 16, 2025

makramkd Jan 16, 2025

makramkd Jan 16, 2025

makramkd Jan 16, 2025

makramkd Jan 16, 2025

AnieeG Jan 16, 2025

AnieeG Jan 16, 2025

cl-sonarqube-production bot commented Jan 16, 2025

github-actions bot commented Jan 16, 2025

		func buildTimelockPerChain(e deployment.Environment, state CCIPOnChainState) map[uint64]*proposalutils.TimelockExecutionContracts {
		func BuildTimelockPerChain(e deployment.Environment, state CCIPOnChainState) map[uint64]*proposalutils.TimelockExecutionContracts {

		func buildProposerPerChain(e deployment.Environment, state CCIPOnChainState) map[uint64]*gethwrappers.ManyChainMultiSig {
		func BuildProposerPerChain(e deployment.Environment, state CCIPOnChainState) map[uint64]*gethwrappers.ManyChainMultiSig {

	changeset2 "github.com/smartcontractkit/chainlink/deployment/common/changeset"
	cscommon "github.com/smartcontractkit/chainlink/deployment/common/changeset"

	// MockLinkAggregatorDescription This is the description of the MockV3Aggregator.sol contract
	// MockLinkAggregatorDescription is the description of the MockV3Aggregator.sol contract

	// MockWETHAggregatorDescription WETH use description from MockETHUSDAggregator.sol
	// MockWETHAggregatorDescription is the description from MockETHUSDAggregator.sol

Improvement in naming changesets #15943

Improvement in naming changesets #15943

Conversation

AnieeG commented Jan 15, 2025 • edited Loading

github-actions bot commented Jan 15, 2025 • edited Loading

AER Report: CI Core ran successfully ✅

AER Report: Operator UI CI ran successfully ✅

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cl-sonarqube-production bot commented Jan 16, 2025

Quality Gate passed

github-actions bot commented Jan 16, 2025

Flakeguard Summary

Found Flaky Tests ❌

Artifacts

AnieeG commented Jan 15, 2025 •

edited

Loading

github-actions bot commented Jan 15, 2025 •

edited

Loading