-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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(systemtests): correct the binary version #23106
Conversation
📝 WalkthroughWalkthroughThe pull request updates the system test documentation by changing the binary name from Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/systemtests/README.md (1)
Line range hint
1-47
: Documentation structure and content is well-organizedThe README provides clear instructions and covers all necessary aspects including setup, execution, and OS-specific considerations. The documentation is well-structured and free from spelling or grammatical errors.
Consider adding:
- A note about the binary version change in the changelog
- A troubleshooting section for common issues
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
systemtests/getting_started.md
(1 hunks)tests/systemtests/README.md
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- systemtests/getting_started.md
🧰 Additional context used
📓 Path-based instructions (1)
tests/systemtests/README.md (2)
Pattern tests/**/*
: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern **/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🔇 Additional comments (2)
tests/systemtests/README.md (2)
34-34
: LGTM! Improved command formatting
The removal of extra whitespace in the test command improves readability while maintaining functionality.
28-28
: Verify binary name consistency across documentation
The binary name change from simd
to simdv2
looks correct, but let's ensure this change is consistently applied across all documentation.
Run the following script to check for any remaining references to the old binary name:
✅ Verification successful
Binary name change is consistent with system test configuration
The change from simd
to simdv2
in the README.md is consistent with the system test configuration. The Makefile explicitly handles this binary name change through the build option flag:
$(if $(findstring v2,$(COSMOS_BUILD_OPTIONS)),--binary=simdv2)
This indicates that when the v2 build option is enabled, the system tests will use simdv2
as the binary name, which aligns with the documentation change in the README.md.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to 'simd' binary
# Exclude git files and binary files to focus on documentation
rg --type-add 'docs:*.{md,txt,rst}' -t docs 'simd(?!v2)'
Length of output: 276
Script:
#!/bin/bash
# Description: Check for any remaining references to 'simd' binary using PCRE2
# Exclude git files and binary files to focus on documentation
rg --pcre2 --type-add 'docs:*.{md,txt,rst}' -t docs 'simd(?!v2)'
# Also search with simple pattern to ensure we don't miss anything
rg --type-add 'docs:*.{md,txt,rst}' -t docs '\bsimd\b'
Length of output: 66191
Script:
#!/bin/bash
# Let's check for any references to the binary name in build or test scripts
rg -g '*.{sh,bash,py}' 'simdv2|simd' tests/systemtests/
Length of output: 57
Script:
#!/bin/bash
# Let's check for any references to the binary name in all files under tests/systemtests
find tests/systemtests -type f -exec grep -l "simd\|simdv2" {} \;
# Also check for any build scripts in the root directory that might reference the binary
find . -maxdepth 1 -type f -name "*.sh" -exec grep -l "simd\|simdv2" {} \;
Length of output: 264
Script:
#!/bin/bash
# Let's examine the content of these files to understand the binary name usage
echo "=== Content of tests/systemtests/README.md ==="
cat tests/systemtests/README.md
echo -e "\n=== Content of tests/systemtests/Makefile ==="
cat tests/systemtests/Makefile
echo -e "\n=== Content of tests/systemtests/upgrade_test.go ==="
cat tests/systemtests/upgrade_test.go
Length of output: 6705
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are examples they are fine to stay as is.
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
in the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
simd
tosimdv2