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

Fix test snapshot exclusion #15727

Merged
merged 11 commits into from
Dec 19, 2024
Merged

Fix test snapshot exclusion #15727

merged 11 commits into from
Dec 19, 2024

Conversation

RensR
Copy link
Contributor

@RensR RensR commented Dec 17, 2024

This PR cleans up the base contracts for all tests, removing the MockRMN in favor of RMNRemote and moving logic further up the inheritance chain. This should result in (minor) compilation gains. This PR also renames the reverts tests to _RevertWhen_ tests so the snapshot excludes them. This was done using regex so not all names are perfect. The ongoing initiative of fixing all test names in any file you touch should continue, but at least we've addresses the snapshot part of it.

This PR

➜  contracts git:(fix-test-naming) forge build --no-cache     
[⠊] Compiling...
[⠊] Compiling 386 files with Solc 0.8.24
[⠆] Solc 0.8.24 finished in 101.08s
Compiler run successful!
➜  contracts git:(fix-test-naming) forge build --no-cache
[⠊] Compiling...
[⠒] Compiling 386 files with Solc 0.8.24
[⠘] Solc 0.8.24 finished in 98.60s
Compiler run successful!

Develop

➜  contracts git:(develop) forge build --no-cache
[⠊] Compiling...
[⠔] Compiling 388 files with Solc 0.8.24
[⠑] Solc 0.8.24 finished in 102.75s
Compiler run successful!
➜  contracts git:(develop) forge build --no-cache
[⠊] Compiling...
[⠢] Compiling 388 files with Solc 0.8.24
[⠰] Solc 0.8.24 finished in 99.21s
Compiler run successful!

It appears to be ~1% faster while improving our test suite.

@RensR RensR requested a review from a team as a code owner December 17, 2024 09:50
Copy link
Contributor

github-actions bot commented Dec 17, 2024

AER Report: CI Core

aer_workflow , commit , Clean Go Tidy & Generate , Detect Changes , Scheduled Run Frequency , Flakeguard Root Project / Get Tests To Run , Core Tests (go_core_tests) , Core Tests (go_core_tests_integration) , GolangCI Lint , Core Tests (go_core_ccip_deployment_tests) , Core Tests (go_core_fuzz) , Core Tests (go_core_race_tests) , test-scripts , Flakeguard Deployment Project , Flakeguard Root Project / Run Tests , Flakeguard Root Project / Report , lint , SonarQube Scan , Flakey Test Detection

1. GolangCI Lint typechecking error:[Golang Lint]

Source of Error:
2024-12-19T11:15:10.4637131Z level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix / does not contain main module or its selected dependencies"
**Why**: This error occurs because the specified directory `/` does not contain the main Go module or its dependencies, which is required for type checking.

Suggested fix: Ensure that the working-directory parameter points to the correct directory containing the Go module.

2. Permission denied for golangci-lint report:[Golang Lint]

Source of Error:
2024-12-19T11:15:10.4638507Z level=error msg="Running error: can't create output for golangci-lint-report.xml: open golangci-lint-report.xml: permission denied"
**Why**: The action does not have the necessary permissions to create or write to the `golangci-lint-report.xml` file.

Suggested fix: Ensure that the action has write permissions to the directory where golangci-lint-report.xml is being created.

3. GolangCI Lint exit code 3:[Golang Lint]

Source of Error:
2024-12-19T11:15:10.4673219Z ##[error]golangci-lint exit with code 3
**Why**: This is a result of the previous errors encountered during the GolangCI Lint run.

Suggested fix: Address the typechecking error and permission issues to resolve this exit code.

4. No such file or directory for golangci-lint report:[Golang Lint]

Source of Error:
2024-12-19T11:15:10.4905981Z cat: .//golangci-lint-report.xml: No such file or directory
**Why**: The `golangci-lint-report.xml` file was not created due to the permission error, leading to this file not being found.

Suggested fix: Fix the permission issue to ensure the file is created successfully.

5. No files found for upload:[Golang Lint]

Source of Error:
2024-12-19T11:15:10.7617785Z ##[warning]No files were found with the provided path: .//golangci-lint-report.xml. No artifacts will be uploaded.
**Why**: This warning is a consequence of the `golangci-lint-report.xml` file not being created due to the earlier permission error.

Suggested fix: Ensure the golangci-lint-report.xml file is created by fixing the permission issue.

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

Copy link
Contributor

github-actions bot commented Dec 17, 2024

Static analysis results are available

Hey @RensR, you can view Slither reports in the job summary here or download them as artifact here.
Please check them before merging and make sure you have addressed all issues.

0xsuryansh
0xsuryansh previously approved these changes Dec 17, 2024
sourceChainSelector: uint64(_randomNum()),
onRampAddress: abi.encode(_randomAddress()),
sourceChainSelector: uint64(vm.randomUint()),
onRampAddress: abi.encode(vm.randomAddress()),
Copy link
Member

Choose a reason for hiding this comment

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

nice!

jhweintraub
jhweintraub previously approved these changes Dec 17, 2024
# Conflicts:
#	contracts/gas-snapshots/ccip.gas-snapshot
#	contracts/src/v0.8/ccip/test/rmn/RMNRemote/RMNRemote.globalCurses.t.sol
#	core/gethwrappers/ccip/generated/rmn_remote/rmn_remote.go
#	core/gethwrappers/ccip/generation/generated-wrapper-dependency-versions-do-not-edit.txt
@RensR RensR dismissed stale reviews from jhweintraub and 0xsuryansh via dd89a85 December 19, 2024 11:13
@cl-sonarqube-production
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@RensR RensR enabled auto-merge December 19, 2024 13:38
@RensR RensR added this pull request to the merge queue Dec 19, 2024
Merged via the queue into develop with commit 72fd6a4 Dec 19, 2024
206 of 208 checks passed
@RensR RensR deleted the fix-test-naming branch December 19, 2024 15:14
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.

3 participants