Skip to content

Introduce a way to add extra argument to library cmd on parametric scenario#4470

Merged
cbeauchesne merged 4 commits intomainfrom
cbeauchesne/parametric-extra-cmd-arg
Apr 4, 2025
Merged

Introduce a way to add extra argument to library cmd on parametric scenario#4470
cbeauchesne merged 4 commits intomainfrom
cbeauchesne/parametric-extra-cmd-arg

Conversation

@cbeauchesne
Copy link
Copy Markdown
Collaborator

Motivation

Allow to add arbitary argument to library container command in parametric scenario

Changes

Add library_extra_command_arguments parameter.

Usage :

import pytest
from utils import scenarios


class Test_stuff:
    @scenarios.parametric
    @pytest.mark.parametrize(
        "library_extra_command_arguments",
        [
            ["--arg1"]
            ["--arg2=12"]
        ],
    )
    def test_stuff(self, test_agent, test_library):
        pass

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes (if something not related to your task is failing, you can ignore it)
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner. We're working on refining the codeowners file quickly.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • If PR title starts with [<language>], double-check that only <language> is impacted by the change
  • No system-tests internal is modified. Otherwise, I have the approval from R&P team
  • CI is green, or failing jobs are not related to this change (and you are 100% sure about this statement)
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added (or removed)?

@cbeauchesne cbeauchesne requested a review from mtoffl01 April 3, 2025 14:24
@cbeauchesne cbeauchesne marked this pull request as ready for review April 3, 2025 14:24
@cbeauchesne cbeauchesne requested review from a team and mabdinur as code owners April 3, 2025 14:24
@cbeauchesne cbeauchesne enabled auto-merge (squash) April 3, 2025 14:24
Copy link
Copy Markdown
Contributor

@mtoffl01 mtoffl01 left a comment

Choose a reason for hiding this comment

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

(Not a blocker) Thanks for adding the SYSTEM_TESTS_EXTRA_COMMAND_ARGUMENTS to the java container script! But is there a good way to mark that the other libraries will have to do this as well, without us having to do it for all libs right now?

@cbeauchesne
Copy link
Copy Markdown
Collaborator Author

I forgot to push a commit ...

@cbeauchesne cbeauchesne requested a review from a team as a code owner April 3, 2025 14:37
Comment on lines +43 to 44
${SYSTEM_TESTS_EXTRA_COMMAND_ARGUMENTS:-} \
-jar target/dd-trace-java-client-1.0.0.jar

This comment was marked as resolved.

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah. And I realized that the feature I'd like to test relies on JVM args, not application args, so we need to keep the command as you have it (ignore my suggestion above). As a user of the library_extra_command_arguments for the java test library, I cannot use --arg1, I need to use a JVM option. Perhaps this should be documented, although I don't know of a good generic way to do so, as it may be different for different languages.

Copy link
Copy Markdown
Collaborator Author

@cbeauchesne cbeauchesne Apr 3, 2025

Choose a reason for hiding this comment

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

you can definitly do something like :

from utils import context

extra_args = ["--default-option"]

if context.library.name == "java":
    extra_args = ["--java-options=1"]
else:
    extra_args = ["--default-option=true"]

class Test_stuff:
    @scenarios.parametric
    @pytest.mark.parametrize(
        "library_extra_command_arguments",
        [
            extra_args,
        ],
    )
    def test_stuff(self, test_agent, test_library):
        pass

@cbeauchesne cbeauchesne disabled auto-merge April 4, 2025 07:56
@cbeauchesne cbeauchesne merged commit 4ed15a0 into main Apr 4, 2025
439 checks passed
@cbeauchesne cbeauchesne deleted the cbeauchesne/parametric-extra-cmd-arg branch April 4, 2025 07:56
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