Skip to content

Conversation

quic-mtharu
Copy link

@quic-mtharu quic-mtharu commented Aug 18, 2025

Fastrpc Test: Enhancing Execution by Eliminating Architecture Dependency

Description: This update simplifies the FastRPC test script by removing the need for architecture-specific dependencies. The goal is to enhance execution and portability across various platforms. This change is motivated by the necessity to streamline test execution, reducing the need for architecture specifications which have previously led to inconsistencies and unnecessary complexity in CI testing environments.

Why this change is needed:

  • Reducing Redundant Detection Logic: The previous script included dynamic architecture detection, which added unnecessary overhead. Removing this logic simplifies the test setup.
  • Default Architecture Sufficiency: Most test environments now default to a stable architecture (e.g., v68), compatible with the majority of FastRPC test cases. Explicit architecture flags are therefore unnecessary for standard runs.
  • Simplified CI Integration: Eliminating architecture-specific flags aligns with our CI enablement goals, allowing tests to run on any target seamlessly.

@ekanshibu
Copy link

Update commit message in some better way. Also update PR description

@smuppand
Copy link
Contributor

The sign-off is missing from the commit. Additionally, please include more details in the commit message explaining why you want to remove the -arch dependency from the test.

log_info "Binary: $FASTBIN"

# ---------- Arch detection if needed ----------
if [ -z "$ARCH" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a specific reason for removing the arch support? Is it not required to run the fastrpc_test?

Copy link
Author

Choose a reason for hiding this comment

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

yes, it's not needed

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, it's not needed

Instead of removing it, simply avoid calling --arch in your .yaml file. It defaults to v68 anyway.

@smuppand
Copy link
Contributor

Along with several others, was merged in 10726ad.

@smuppand smuppand closed this Aug 26, 2025
@ekanshibu
Copy link

ekanshibu commented Aug 26, 2025

Along with several others, was merged in 10726ad.

I don't think this is the right way to close a PR, this is a public open repo. If there are any concerns, pls post it as a comment to the PR and let the change owner fix that. If you have any additional change that you want to bring, please bring it on top of this PR.

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