Skip to content

For loop audit#1117

Merged
MSevey merged 2 commits into
mainfrom
sevey/background-threads
Aug 8, 2023
Merged

For loop audit#1117
MSevey merged 2 commits into
mainfrom
sevey/background-threads

Conversation

@MSevey

@MSevey MSevey commented Jul 31, 2023

Copy link
Copy Markdown
Contributor

Overview

I did a quick audit of the code for infinite for loops to catch any remaining places where we might be hanging after a shutdown.

This PR adds some comments to explain some test helpers and adds two select statements that watch for cancellation of subscriptions to help with a cleaner shutdown.

closes #1069

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@MSevey MSevey added the T:code-hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity. label Jul 31, 2023
@MSevey MSevey self-assigned this Jul 31, 2023
@codecov

codecov Bot commented Jul 31, 2023

Copy link
Copy Markdown

Codecov Report

Patch coverage: 67.07% and project coverage change: +0.51% 🎉

Comparison is base (9460a18) 55.52% compared to head (5400bbe) 56.03%.
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1117      +/-   ##
==========================================
+ Coverage   55.52%   56.03%   +0.51%     
==========================================
  Files          63       61       -2     
  Lines        6808     6555     -253     
==========================================
- Hits         3780     3673     -107     
+ Misses       2622     2490     -132     
+ Partials      406      392      -14     
Files Changed Coverage Δ
config/config.go 89.65% <ø> (-0.67%) ⬇️
rpc/json/test_helpers.go 100.00% <ø> (ø)
block/manager.go 9.43% <8.33%> (+0.54%) ⬆️
state/txindex/indexer_service.go 63.33% <51.51%> (+0.37%) ⬆️
node/full.go 64.57% <100.00%> (-0.01%) ⬇️
node/light.go 60.00% <100.00%> (+9.54%) ⬆️
node/test_helpers.go 78.94% <100.00%> (-1.70%) ⬇️
state/executor.go 55.52% <100.00%> (+5.40%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MSevey MSevey enabled auto-merge August 1, 2023 13:44

@Manav-Aggarwal Manav-Aggarwal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@MSevey MSevey added this pull request to the merge queue Aug 8, 2023
Merged via the queue into main with commit 58ab158 Aug 8, 2023
@MSevey MSevey deleted the sevey/background-threads branch August 8, 2023 13:40
Manav-Aggarwal pushed a commit that referenced this pull request Sep 6, 2023
<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.
-->

## Overview
I did a quick audit of the code for infinite for loops to catch any
remaining places where we might be hanging after a shutdown.

This PR adds some comments to explain some test helpers and adds two
select statements that watch for cancellation of subscriptions to help
with a cleaner shutdown.

<!-- 
Please provide an explanation of the PR, including the appropriate
context,
background, goal, and rationale. If there is an issue with this
information,
please provide a tl;dr and link the issue. 
-->

closes #1069 

## Checklist

<!-- 
Please complete the checklist to ensure that the PR is ready to be
reviewed.

IMPORTANT:
PRs should be left in Draft until the below checklist is completed.
-->

- [ ] New and updated code has appropriate documentation
- [ ] New and updated code has new and/or updated testing
- [ ] Required CI checks are passing
- [ ] Visual proof for any user facing features like CLI or
documentation updates
- [ ] Linked issues closed with keywords
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T:code-hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EPIC: Fast Clean Shutdowns and Testing Design

4 participants