Skip to content
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.

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

Ccip-4681 lint fix for deployment/ccip #15783

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from
Open

Conversation

AnieeG
Copy link
Contributor

@AnieeG AnieeG commented Dec 19, 2024

Requires

Supports

Copy link
Contributor

Flakeguard Summary

Ran new or updated tests between develop and 971ae72 (ccip-4681-lint-fix).

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
Test_UpdateChainConfigs 0.00% false false false 3 0 3 0 github.com/smartcontractkit/chainlink/deployment/ccip/changeset false 31.243333333s @smartcontractkit/ccip, @smartcontractkit/core, @smartcontractkit/deployment-automation
Test_UpdateChainConfigs/MCMS_disabled 0.00% false false false 3 0 3 0 github.com/smartcontractkit/chainlink/deployment/ccip/changeset false 14.93s @smartcontractkit/ccip, @smartcontractkit/core, @smartcontractkit/deployment-automation
Test_UpdateChainConfigs/MCMS_enabled 0.00% false false false 3 0 3 0 github.com/smartcontractkit/chainlink/deployment/ccip/changeset false 16.313333333s @smartcontractkit/ccip, @smartcontractkit/core, @smartcontractkit/deployment-automation
TestE2ELegacy 0.00% false false false 3 0 3 0 github.com/smartcontractkit/chainlink/deployment/ccip/changeset/v1_5 false 6m12.083333333s @smartcontractkit/test-tooling-team, @smartcontractkit/core

Artifacts

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

Copy link
Contributor

AER Report: CI Core

aer_workflow , commit , Detect Changes , Clean Go Tidy & Generate , Scheduled Run Frequency , Flakeguard Root Project / Get Tests To Run , GolangCI Lint (integration-tests) , Core Tests (go_core_tests) , GolangCI Lint (deployment) , Core Tests (go_core_tests_integration) , test-scripts , Core Tests (go_core_ccip_deployment_tests) , Core Tests (go_core_fuzz) , Core Tests (go_core_race_tests) , Flakeguard Deployment Project / Get Tests To Run , Flakeguard Root Project / Run Tests , Flakeguard Root Project / Report , Flakeguard Deployment Project / Run Tests (github.com/smartcontractkit/chainlink/deployment/ccip/changeset, ubuntu-latest) , Flakeguard Deployment Project / Run Tests (github.com/smartcontractkit/chainlink/deployment/ccip/changeset/v1_5, ubuntu-latest) , Flakeguard Deployment Project / Run Tests (github.com/smartcontractkit/chainlink/deployment/ccip/view/v1_2, ubuntu-latest) , Flakeguard Deployment Project / Run Tests (github.com/smartcontractkit/chainlink/deployment/ccip/view/v1_5, ubuntu-latest) , Flakeguard Deployment Project / Run Tests (github.com/smartcontractkit/chainlink/deployment/ccip/view/v1_6, ubuntu-latest) , lint , SonarQube Scan , Flakey Test Detection , Flakeguard Deployment Project / Report

1. Timeout error during log pruning:[Run tests]

Source of Error:
logger.go:146: 2024-12-19T21:35:25.143Z	ERROR	EVM.90000001.LogPoller	logpoller/log_poller.go:1178	Unable to find excess logs for pruning	{"version": "unset@unset", "err": "timeout: context canceled"}
logger.go:146: 2024-12-19T21:35:25.143Z	ERROR	EVM.90000001.LogPoller	logpoller/log_poller.go:714	unable to prune expired logs	{"version": "unset@unset", "err": "timeout: context canceled"}
**Why**: The log poller encountered a timeout while attempting to prune expired logs, resulting in a context cancellation error. This indicates that the operation took longer than the allowed time.

Suggested fix: Increase the timeout duration for the log pruning operation to ensure it has sufficient time to complete. Alternatively, optimize the log pruning process to execute more quickly.

2. Test failure in github.com/smartcontractkit/chainlink/deployment/ccip/changeset/v1_5:[Run tests]

Source of Error:
Run tests	2024-12-19T21:35:29.3343383Z FAIL
Run tests	2024-12-19T21:35:29.3343759Z FAIL	github.com/smartcontractkit/chainlink/deployment/ccip/changeset/v1_5	329.431s
**Why**: The test suite for `github.com/smartcontractkit/chainlink/deployment/ccip/changeset/v1_5` failed, but the specific reason for the failure is not detailed in the provided logs.

Suggested fix: Investigate the detailed test logs for github.com/smartcontractkit/chainlink/deployment/ccip/changeset/v1_5 to identify the specific test case(s) that failed and address the underlying issues.

3. Git diff detected changes after tidy:[Ensure clean after tidy]

Source of Error:
Ensure clean after tidy	2024-12-19T21:36:21.9219581Z diff --git a/deployment/go.mod b/deployment/go.mod
Ensure clean after tidy	2024-12-19T21:36:21.9221120Z index 9eb3a02d57..f850dc8269 100644
Ensure clean after tidy	2024-12-19T21:36:21.9222131Z --- a/deployment/go.mod
Ensure clean after tidy	2024-12-19T21:36:21.9223023Z +++ b/deployment/go.mod
Ensure clean after tidy	2024-12-19T21:36:21.9224332Z @@ -36,7 +36,6 @@ require (
Ensure clean after tidy	2024-12-19T21:36:21.9225341Z 	github.com/stretchr/testify v1.9.0
Ensure clean after tidy	2024-12-19T21:36:21.9226389Z 	github.com/test-go/testify v1.1.4
Ensure clean after tidy	2024-12-19T21:36:21.9227602Z 	github.com/testcontainers/testcontainers-go v0.34.0
Ensure clean after tidy	2024-12-19T21:36:21.9228825Z -	go.uber.org/multierr v1.11.0
Ensure clean after tidy	2024-12-19T21:36:21.9229823Z 	go.uber.org/zap v1.27.0
Ensure clean after tidy	2024-12-19T21:36:21.9231091Z 	golang.org/x/exp v0.0.0-20241009180824-f66d83c29e7c
Ensure clean after tidy	2024-12-19T21:36:21.9232199Z 	golang.org/x/oauth2 v0.23.0
Ensure clean after tidy	2024-12-19T21:36:21.9233127Z @@ -490,6 +489,7 @@ require (
Ensure clean after tidy	2024-12-19T21:36:21.9234307Z 	go.starlark.net v0.0.0-20230525235612-a134d8f9ddca // indirect
Ensure clean after tidy	2024-12-19T21:36:21.9236530Z 	go.uber.org/atomic v1.11.0 // indirect
Ensure clean after tidy	2024-12-19T21:36:21.9237625Z 	go.uber.org/goleak v1.3.0 // indirect
Ensure clean after tidy	2024-12-19T21:36:21.9238766Z +	go.uber.org/multierr v1.11.0 // indirect
Ensure clean after tidy	2024-12-19T21:36:21.9239894Z 	go.uber.org/ratelimit v0.3.1 // indirect
Ensure clean after tidy	2024-12-19T21:36:21.9241143Z 	go4.org/netipx v0.0.0-20230125063823-8449b0a6169f // indirect
Ensure clean after tidy	2024-12-19T21:36:21.9242344Z 	golang.org/x/arch v0.11.0 // indirect
**Why**: The `go.mod` file was modified after running `go mod tidy`, indicating that dependencies were added or removed.

Suggested fix: Ensure that the go.mod file is in the correct state before running the tidy command. If changes are expected, commit them. If not, investigate why go mod tidy is making unexpected changes.

go func() {
require.NoError(t, wg.Wait())
close(done)
errFound <- wg.Wait()
Copy link
Contributor

Choose a reason for hiding this comment

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

close(errFound) is still valid here

}
return nil
}

func (tc *TestConfigs) MustSetEnvTypeOrDefault(t *testing.T) {
envType := os.Getenv(ENVTESTTYPE)
if envType == "" || envType == string(Memory) {
switch envType {
case string(Memory):
Copy link
Contributor

Choose a reason for hiding this comment

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

You lost the default handling:

Suggested change
case string(Memory):
case "", string(Memory):

@@ -2,6 +2,7 @@ package changeset

import (
"context"
"errors"
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like pkg/errors also has a New function, so no need to import both of these.

@@ -89,3 +89,10 @@ func (params OCRParameters) Validate() error {
}
return nil
}

func MustGetInt(num uint64) int {
if num > uint64(int(^uint(0)>>1)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if num > uint64(int(^uint(0)>>1)) {
if num > math.MaxInt {

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.

2 participants