Skip to content

[tmpnet] Ensure tmpnet methods always have a logger #3893

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

Merged
merged 1 commit into from
Apr 18, 2025

Conversation

maru-ava
Copy link
Contributor

@maru-ava maru-ava commented Apr 17, 2025

PR Chain: tmpnet+kube

This PR chain enables tmpnet to deploy temporary networks to Kubernetes. Early PRs refactor tmpnet to support the addition in #3615 of a new tmpnet node runtime for kube.

Why this should be merged

Previously individual tmpnet methods accepted a log argument where needed. Adding a log field to tmpnet.Network ensures that methods will always have access to a logger without the caller having to provide it past network initialization.

How this was tested

CI

Need to be documented in RELEASES.md?

N/A

TODO

@maru-ava maru-ava added the tooling Build, test and development tooling label Apr 17, 2025
@github-project-automation github-project-automation bot moved this to Backlog 🗄️ in avalanchego Apr 17, 2025
@maru-ava maru-ava moved this from Backlog 🗄️ to In Review 👀 in avalanchego Apr 17, 2025
@maru-ava maru-ava force-pushed the tmpnet-ensure-logger branch from e05a502 to ed26001 Compare April 17, 2025 20:50
@maru-ava maru-ava removed the request for review from StephenButtolph April 17, 2025 21:23
@maru-ava maru-ava self-assigned this Apr 17, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR embeds a logger in tmpnet.Network so that all tmpnet methods have consistent logging without requiring an explicit logger parameter in every call. Key changes include:

  • Removing the logger parameter from method signatures in tmpnet and updating internal calls to use the network's logger field.
  • Updating test files and helper functions to call the new method signatures and to pass a logger only during network initialization.
  • Refactoring functions like ReadNetwork, StartNode, RestartNode, and StopNetwork to accommodate these changes.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/upgrade/upgrade_test.go Updated StartNode call to remove logger argument.
tests/fixture/tmpnet/tmpnetctl/main.go Refactored StopNetwork call to create and pass a logger.
tests/fixture/tmpnet/process_runtime.go Refactored ProcessRuntime.Start to obtain the logger from the network field.
tests/fixture/tmpnet/node.go Removed logger parameter from Node.Start and updated interface accordingly.
tests/fixture/tmpnet/network_test.go Updated ReadNetwork call to pass a NoLog logger instance.
tests/fixture/tmpnet/network.go Integrated a logger into the Network struct and refactored all logger usage.
tests/fixture/e2e/helpers.go Updated StartNode invocation to align with the new signature.
tests/fixture/e2e/env.go Revised environment initialization to supply a logger where needed.
tests/e2e/e2e_test.go Adapted shared test environment initialization to use the new TestContext.

Base automatically changed from tmpnet-flagsmap-string-values to master April 18, 2025 19:54
Previously individual tmpnet methods accepted a log argument where
needed. Adding a log field to `tmpnet.Network` ensures that methods
will always have access to a logger without the caller having to
provide it past network initialization.
@maru-ava maru-ava force-pushed the tmpnet-ensure-logger branch from ed26001 to aa5189d Compare April 18, 2025 20:12
@maru-ava
Copy link
Contributor Author

Rebased

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tooling Build, test and development tooling
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants