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

Problem: use prebuilt image in testground #1480

Merged
merged 6 commits into from
Jun 21, 2024

Conversation

yihuang
Copy link
Collaborator

@yihuang yihuang commented Jun 21, 2024

Solution:

  • build image manually, don't use testground builders at all, and don't need the custom nix builder at all.
  • update doc.

👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻

PR Checklist:

  • Have you read the CONTRIBUTING.md?
  • Does your PR follow the C4 patch requirements?
  • Have you rebased your work on top of the latest master?
  • Have you checked your code compiles? (make)
  • Have you included tests for any non-trivial functionality?
  • Have you checked your code passes the unit tests? (make test)
  • Have you checked your code formatting is correct? (go fmt)
  • Have you checked your basic code style is fine? (golangci-lint run)
  • If you added any dependencies, have you checked they do not contain any known vulnerabilities? (go list -json -m all | nancy sleuth)
  • If your changes affect the client infrastructure, have you run the integration test?
  • If your changes affect public APIs, does your PR follow the C4 evolution of public contracts?
  • If your code changes public APIs, have you incremented the crate version numbers and documented your changes in the CHANGELOG.md?
  • If you are contributing for the first time, please read the agreement in CONTRIBUTING.md now and add a comment to this pull request stating that your PR is in accordance with the Developer's Certificate of Origin.

Thank you for your code, it's appreciated! :)

Summary by CodeRabbit

  • Documentation

    • Updated build image instructions and prerequisites in the README.
    • Added steps for building and loading images locally using nix.
    • Revised instructions for running the Testground daemon and test plans.
  • Bug Fixes

    • Filtered out <jemalloc>: warning messages from command output in cli.py.
  • Refactor

    • Changed builder configuration in manifest.toml from docker:nix to docker:go.
    • Removed and replaced builder fields in local.toml with artifact fields for validators and fullnodes.

Solution:
- build image manually, don't use testground builders at all.
- update doc.
@yihuang yihuang requested a review from a team as a code owner June 21, 2024 03:27
@yihuang yihuang requested review from devashishdxt and thomas-nguy and removed request for a team June 21, 2024 03:27
Copy link
Contributor

coderabbitai bot commented Jun 21, 2024

Warning

Rate limit exceeded

@yihuang has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 7 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Commits

Files that changed from the base of the PR and between 91833e6 and 542c92e.

Walkthrough

Recent updates to the testground project include significant enhancements to the README.md, focusing on streamlining the process for building and using images locally. Additionally, updates to the cli.py improve usability by filtering out unwanted <jemalloc>: warnings. Modifications in local.toml and manifest.toml reflect changes to the builder configuration and the addition of artifact fields.

Changes

File Change Summary
testground/README.md Updated build image section, instructions for local image building, modified test run instructions.
testground/benchmark/cli.py Added filtering of <jemalloc>: warnings from stdout output.
testground/benchmark/compositions/local.toml Removed builder field, added artifact field for groups validators and fullnodes.
testground/benchmark/manifest.toml Changed builder configuration from "docker:nix" to "docker:go".

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Cli
    participant TestgroundDaemon

    User->>Cli: Execute command
    Cli->>Cli: Filter `<jemalloc>:` warnings
    Cli->>User: Return filtered stdout

    User->>TestgroundDaemon: Build image with nix
    TestgroundDaemon->>User: Image built and loaded
Loading

Poem

In the realm of code so bound,
The Testground laughs without a sound.
Images built with nix's grace,
Warning filters keep their place.
Builders change, configs align,
CodeRabbit's work is simply fine!


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@yihuang yihuang requested a review from mmsqe June 21, 2024 03:32
testground/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 903b590 and 8625515.

Files selected for processing (4)
  • testground/README.md (2 hunks)
  • testground/benchmark/benchmark/cli.py (1 hunks)
  • testground/benchmark/compositions/local.toml (2 hunks)
  • testground/benchmark/manifest.toml (1 hunks)
Files skipped from review due to trivial changes (1)
  • testground/benchmark/manifest.toml
Additional context used
Markdownlint
testground/README.md

71-71: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines


34-34: null (MD014, commands-show-output)
Dollar signs used before commands without showing output


44-44: null (MD014, commands-show-output)
Dollar signs used before commands without showing output


50-50: null (MD014, commands-show-output)
Dollar signs used before commands without showing output


58-58: null (MD014, commands-show-output)
Dollar signs used before commands without showing output


5-5: null (MD027, no-multiple-space-blockquote)
Multiple spaces after blockquote symbol

Additional comments not posted (5)
testground/benchmark/compositions/local.toml (1)

27-28: Ensure the artifact URL is correct and accessible for the fullnodes group.

Similar to the validators group, ensure that the specified Docker image is correct and accessible.

testground/README.md (3)

3-14: Refine Build Image section for clarity and completeness.

The new prerequisites and instructions for building images locally using nix are clear. Ensure all necessary steps are included and accurately described.

Tools
Markdownlint

5-5: null (MD027, no-multiple-space-blockquote)
Multiple spaces after blockquote symbol


31-31: Ensure clarity in the instructions for running the Testground Daemon.

The command provided should be clear and functional. Verify that the environment variable TESTGROUND_HOME is correctly set and used.


Line range hint 58-58: Ensure macOS instructions are specific and accurate.

The instructions for macOS users using colima should be verified for accuracy and specificity. Ensure that the symbolic link and mount points are correctly configured.

Verification successful

The output indicates that the symbolic link is not configured. This suggests that either the instructions in the README.md are incorrect or the symbolic link was not created properly. To proceed, we need to verify the exact instructions provided in the README.md file to ensure they are correctly guiding the user to create this symbolic link.


The instructions in the README.md file to create a symbolic link from $HOME/.colima/docker.sock to /var/run/docker.sock are correct. However, the symbolic link was not found during the script execution. This might indicate an issue with the execution environment or user permissions.

To ensure the instructions are clear and specific, we need to verify if there are any missing steps or prerequisites that need to be addressed in the README.md. This includes checking for any setup steps required for colima and Docker on macOS.


The extracted macOS section from the README.md file includes instructions to create the symbolic link and mount the related directories into the virtual machine. The instructions appear to be correct and specific.

Given that the symbolic link was not found during the script execution, it is possible that the issue lies in the execution environment or user permissions rather than the instructions themselves.

  • Ensure you have the necessary permissions to create the symbolic link.
  • Verify that $HOME/.colima/docker.sock exists before creating the symbolic link.

If these conditions are met, the instructions should work as intended.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the macOS instructions for accuracy.

# Test: Check if the symbolic link and mount points are correctly configured.
test -L /var/run/docker.sock && echo "Symbolic link is configured" || echo "Symbolic link is not configured"

Length of output: 103



Script:

#!/bin/bash
# Extract the relevant section from the README.md file to verify the instructions for macOS users using colima.
sed -n '58p' testground/README.md

Length of output: 93



Script:

#!/bin/bash
# Extract the entire macOS section from the README.md file to verify if there are any missing steps or prerequisites.
sed -n '50,70p' testground/README.md

Length of output: 503

Tools
Markdownlint

5-5: null (MD027, no-multiple-space-blockquote)
Multiple spaces after blockquote symbol

testground/benchmark/benchmark/cli.py (1)

12-22: Review the updated filtering logic in raw method.

The addition of logic to filter out <jemalloc>: warning messages is a good improvement. Ensure that this change does not inadvertently filter out necessary information from other stdout messages.

testground/benchmark/compositions/local.toml Show resolved Hide resolved
testground/README.md Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Outside diff range and nitpick comments (2)
testground/README.md (2)

Line range hint 38-49: Clarify the path in 'Run Test Plan' section.

The paths provided in the instructions need to be accurate to prevent confusion. Ensure that the path reflects the correct location of local.toml and other resources.

- $ TESTGROUND_HOME=$PWD/data testground plan import --from /path/to/cronos/testground/benchmark
+ $ TESTGROUND_HOME=$PWD/data testground plan import --from testground/benchmark/compositions
- $ testground run composition -f /path/to/cronos/testground/benchmark/compositions/local.toml --wait
+ $ testground run composition -f testground/benchmark/compositions/local.toml --wait
Tools
Markdownlint

33-33: null (MD014, commands-show-output)
Dollar signs used before commands without showing output


Line range hint 70-70: Remove unnecessary blank lines.

There are multiple consecutive blank lines which should be removed to maintain consistency in document formatting.

- 
- 
Tools
Markdownlint

5-5: null (MD027, no-multiple-space-blockquote)
Multiple spaces after blockquote symbol

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8625515 and 91833e6.

Files selected for processing (3)
  • testground/README.md (2 hunks)
  • testground/benchmark/compositions/local.toml (2 hunks)
  • testground/benchmark/manifest.toml (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • testground/benchmark/compositions/local.toml
  • testground/benchmark/manifest.toml
Additional context used
Markdownlint
testground/README.md

70-70: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines


33-33: null (MD014, commands-show-output)
Dollar signs used before commands without showing output


43-43: null (MD014, commands-show-output)
Dollar signs used before commands without showing output


49-49: null (MD014, commands-show-output)
Dollar signs used before commands without showing output


57-57: null (MD014, commands-show-output)
Dollar signs used before commands without showing output


5-5: null (MD027, no-multiple-space-blockquote)
Multiple spaces after blockquote symbol

Additional comments not posted (3)
testground/README.md (3)

3-15: Updated Build Image instructions are clear and detailed.

The revised section on building images provides clear instructions for both general and macOS-specific scenarios. The use of nix for building and the steps to load the Docker image are well-documented. This should help users in setting up their environments correctly.

Tools
Markdownlint

5-5: null (MD027, no-multiple-space-blockquote)
Multiple spaces after blockquote symbol


Line range hint 17-29: Installation instructions for Testground are clearly outlined.

The steps to install Testground, including cloning the repository and building dependencies, are well explained. This ensures that even users unfamiliar with the process can follow along without issues.

Tools
Markdownlint

5-5: null (MD027, no-multiple-space-blockquote)
Multiple spaces after blockquote symbol


Line range hint 50-63: Instructions for macOS users are helpful.

The additional instructions for macOS users regarding colima and Docker socket setup are useful. This ensures that macOS users can configure their environments appropriately.

Tools
Markdownlint

5-5: null (MD027, no-multiple-space-blockquote)
Multiple spaces after blockquote symbol

testground/README.md Outdated Show resolved Hide resolved
testground/README.md Show resolved Hide resolved
testground/README.md Outdated Show resolved Hide resolved
@yihuang yihuang enabled auto-merge June 21, 2024 05:47
@yihuang yihuang added this pull request to the merge queue Jun 21, 2024
Merged via the queue into crypto-org-chain:main with commit 3a0e503 Jun 21, 2024
37 checks passed
@yihuang yihuang deleted the fix-warning-message branch June 21, 2024 06:00
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