Updated comments in features

Author: tisnik

tests/e2e/features/environment.py

@@ -50,7 +50,25 @@ def _fetch_models_from_service() -> dict:
 
 
 def before_all(context: Context) -> None:
-    """Run before and after the whole shooting match."""
+    """Run before and after the whole shooting match.
+
+    Initialize global test environment before the test suite runs.
+
+    Sets context.deployment_mode from the E2E_DEPLOYMENT_MODE environment
+    variable (default "server") and context.is_library_mode accordingly.
+
+    Attempts to detect a default LLM model and provider via
+    _fetch_models_from_service() and stores results in context.default_model
+    and context.default_provider; if detection fails, falls back to
+    "gpt-4-turbo" and "openai".
+
+    Parameters:
+        context (Context): Behave context into which this function writes:
+            - deployment_mode (str): "server" or "library".
+            - is_library_mode (bool): True when deployment_mode is "library".
+            - default_model (str): Detected model id or fallback model.
+            - default_provider (str): Detected provider id or fallback provider.
+    """
     # Detect deployment mode from environment variable
     context.deployment_mode = os.getenv("E2E_DEPLOYMENT_MODE", "server").lower()
     context.is_library_mode = context.deployment_mode == "library"
@@ -76,7 +94,18 @@ def before_all(context: Context) -> None:
 
 
 def before_scenario(context: Context, scenario: Scenario) -> None:
-    """Run before each scenario is run."""
+    """Run before each scenario is run.
+
+    Prepare scenario execution by skipping scenarios based on tags and
+    selecting a scenario-specific configuration.
+
+    Skips the scenario if it has the `skip` tag, if it has the `local` tag
+    while the test run is not in local mode, or if it has
+    `skip-in-library-mode` when running in library mode. When the scenario is
+    tagged with `InvalidFeedbackStorageConfig` or `NoCacheConfig`, sets
+    `context.scenario_config` to the appropriate configuration file path for
+    the current deployment mode (library-mode or server-mode).
+    """
     if "skip" in scenario.effective_tags:
         scenario.skip("Marked with @skip")
         return
@@ -100,7 +129,31 @@ def before_scenario(context: Context, scenario: Scenario) -> None:
 
 
 def after_scenario(context: Context, scenario: Scenario) -> None:
-    """Run after each scenario is run."""
+    """Run after each scenario is run.
+
+    Perform per-scenario teardown: restore scenario-specific configuration and,
+    in server mode, attempt to restart and verify the Llama Stack container if
+    it was previously running.
+
+    If the scenario used an alternate feedback storage or no-cache
+    configuration, the original feature configuration is restored and the
+    lightspeed-stack container is restarted. When not running in library mode
+    and the context indicates the Llama Stack was running before the scenario,
+    this function attempts to start the llama-stack container and polls its
+    health endpoint until it becomes healthy or a timeout is reached.
+
+    Parameters:
+        context (Context): Behave test context. Expected attributes used here include:
+            - feature_config: path to the feature-level configuration to restore.
+            - is_library_mode (bool): whether tests run in library mode.
+            - llama_stack_was_running (bool, optional): whether llama-stack was
+              running before the scenario.
+            - hostname_llama, port_llama (str/int, optional): host and port
+              used for the llama-stack health check.
+        scenario (Scenario): Behave scenario whose tags determine which
+        scenario-specific teardown actions to run (e.g.,
+        "InvalidFeedbackStorageConfig", "NoCacheConfig").
+    """
     if "InvalidFeedbackStorageConfig" in scenario.effective_tags:
         switch_config(context.feature_config)
         restart_container("lightspeed-stack")
@@ -161,7 +214,10 @@ def after_scenario(context: Context, scenario: Scenario) -> None:
 
 
 def before_feature(context: Context, feature: Feature) -> None:
-    """Run before each feature file is exercised."""
+    """Run before each feature file is exercised.
+
+    Prepare per-feature test environment and apply feature-specific configuration.
+    """
     if "Authorized" in feature.tags:
         mode_dir = "library-mode" if context.is_library_mode else "server-mode"
         context.feature_config = (
@@ -178,7 +234,11 @@ def before_feature(context: Context, feature: Feature) -> None:
 
 
 def after_feature(context: Context, feature: Feature) -> None:
-    """Run after each feature file is exercised."""
+    """Run after each feature file is exercised.
+
+    Perform feature-level teardown: restore any modified configuration and
+    clean up feedback conversations.
+    """
     if "Authorized" in feature.tags:
         switch_config(context.default_config_backup)
         restart_container("lightspeed-stack")

tests/e2e/features/steps/auth.py

@@ -8,7 +8,11 @@
 
 @given("I set the Authorization header to {header_value}")
 def set_authorization_header_custom(context: Context, header_value: str) -> None:
-    """Set a custom Authorization header value."""
+    """Set a custom Authorization header value.
+
+    Parameters:
+        header_value (str): The value to set for the `Authorization` header.
+    """
     if not hasattr(context, "auth_headers"):
         context.auth_headers = {}
     context.auth_headers["Authorization"] = header_value
@@ -29,6 +33,10 @@ def access_rest_api_endpoint_post(
     """Send POST HTTP request with payload in the endpoint as parameter to tested service.
 
     The response is stored in `context.response` attribute.
+
+    Parameters:
+        endpoint (str): Endpoint path to call; will be normalized.
+        user_id (str): Value used for the `user_id` query parameter (surrounding quotes are removed).
     """
     endpoint = normalize_endpoint(endpoint)
     user_id = user_id.replace('"', "")

tests/e2e/features/steps/common.py

@@ -7,7 +7,14 @@
 
 @given("The service is started locally")
 def service_is_started_locally(context: Context) -> None:
-    """Check the service status."""
+    """Check the service status.
+
+    Populate the Behave context with local service endpoint values read from
+    environment variables.
+
+    Parameters:
+        context (Context): Behave context object to receive the endpoint attributes.
+    """
     assert context is not None
     context.hostname = os.getenv("E2E_LSC_HOSTNAME", "localhost")
     context.port = os.getenv("E2E_LSC_PORT", "8080")
@@ -17,5 +24,15 @@ def service_is_started_locally(context: Context) -> None:
 
 @given("The system is in default state")
 def system_in_default_state(context: Context) -> None:
-    """Check the default system state."""
+    """Check the default system state.
+
+    Ensure the Behave test context is present for steps that assume the system
+    is in its default state.
+
+    Parameters:
+        context (Context): Behave Context instance used to store and share test state.
+
+    Raises:
+        AssertionError: If `context` is None.
+    """
     assert context is not None

tests/e2e/features/steps/health.py

@@ -8,7 +8,22 @@
 
 @given("The llama-stack connection is disrupted")
 def llama_stack_connection_broken(context: Context) -> None:
-    """Break llama_stack connection by stopping the container."""
+    """Break llama_stack connection by stopping the container.
+
+    Disrupts the Llama Stack service by stopping its Docker container and
+    records whether it was running.
+
+    Checks whether the Docker container named "llama-stack" is running; if it
+    is, stops the container, waits briefly for the disruption to take effect,
+    and sets `context.llama_stack_was_running` to True so callers can restore
+    state later. If the container is not running, the flag remains False. On
+    failure to run Docker commands, prints a warning message describing the
+    error.
+
+    Parameters:
+        context (behave.runner.Context): Behave context used to store
+        `llama_stack_was_running` and share state between steps.
+    """
     # Store original state for restoration
     context.llama_stack_was_running = False
 
@@ -39,6 +54,12 @@ def llama_stack_connection_broken(context: Context) -> None:
 
 @given("the service is stopped")
 def stop_service(context: Context) -> None:
-    """Stop service."""
+    """Stop service.
+
+    Stop a service used by the current test scenario.
+
+    Parameters:
+        context (Context): Behave step context carrying scenario state and configuration.
+    """
     # TODO: add step implementation
     assert context is not None