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

OnBoarding: de-parametrize #3960

Merged

Conversation

robertomonteromiguel
Copy link
Collaborator

Motivation

we no longer run multiple virtual machines by scenario. We don't need parametrize the tests. The current VM data is stored in the scenario.

Changes

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)?

Copy link
Collaborator

@cbeauchesne cbeauchesne left a comment

Choose a reason for hiding this comment

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

plus naming

@@ -52,7 +52,7 @@ def setup_install_weblog_running(self):

@features.ssi_guardrails
@bug(
condition="centos-7" in context.scenario.weblog_variant and context.scenario.library.library == "java",
condition="centos-7" in context.weblog_variant and context.scenario.library.library == "java",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be working in theory:

Suggested change
condition="centos-7" in context.weblog_variant and context.scenario.library.library == "java",
condition="centos-7" in context.weblog_variant and context.library == "java",

@@ -19,6 +19,7 @@

import socket
import time
from utils.tools import logger


def wait_for_port(port: int, host: str = "localhost", timeout: float = 5.0):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probaly a better name : wait_for_accepting_connection

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for the next pr

@cbeauchesne cbeauchesne force-pushed the robertomonteromiguel/onboarding_deparametrize branch from 4638612 to 897ca43 Compare February 11, 2025 14:09
@robertomonteromiguel robertomonteromiguel marked this pull request as ready for review February 12, 2025 08:55
@robertomonteromiguel robertomonteromiguel requested a review from a team as a code owner February 12, 2025 08:55
@robertomonteromiguel robertomonteromiguel merged commit b239ef9 into main Feb 12, 2025
418 of 434 checks passed
@robertomonteromiguel robertomonteromiguel deleted the robertomonteromiguel/onboarding_deparametrize branch February 12, 2025 08:56
robertomonteromiguel added a commit that referenced this pull request Mar 17, 2025
Co-authored-by: Charles de Beauchesne <[email protected]>
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