Skip to content

Commit

Permalink
fix: test matrix 0426 (#3032)
Browse files Browse the repository at this point in the history
# Description

Fix below tests:
+ test_validate_tool_func_result
  + str(enum) is not equal to enum.value now
+ test_get_token_with_sovereign_credential
  + multiple `with` is not supported in python 3.8
+ test_search_line_runs_with_tokens
  + Python 3.9+ is required on Windows to support json_extract
+ test_get_connection_by_provider
+ meet error if using relative path to cwd for PFSOperation import (and
actually this is not recommended)
+ test_executor_exec_node_fail
  + Python 3.11 have an extra line of stack trace
+ test_executor_exec_line_fail
  + Python 3.11 have an extra line of stack trace

# All Promptflow Contribution checklist:
- [ ] **The pull request does not introduce [breaking changes].**
- [ ] **CHANGELOG is updated for new features, bug fixes or other
significant changes.**
- [ ] **I have read the [contribution guidelines](../CONTRIBUTING.md).**
- [ ] **Create an issue and link to the pull request to get dedicated
review from promptflow team. Learn more: [suggested
workflow](../CONTRIBUTING.md#suggested-workflow).**

## General Guidelines and Best Practices
- [ ] Title of the pull request is clear and informative.
- [ ] There are a small number of commits, each of which have an
informative message. This means that previously merged commits do not
appear in the history of the PR. For more information on cleaning up the
commits in your PR, [see this
page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md).

### Testing Guidelines
- [ ] Pull request includes test coverage for the included changes.

(cherry picked from commit f00b5ca)
  • Loading branch information
elliotzh committed Apr 26, 2024
1 parent ebf7faa commit 9f616ff
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import mock
import pytest
from _constants import PROMPTFLOW_ROOT
from flask.app import Flask


Expand All @@ -21,8 +22,16 @@ def pfs_op(app: Flask):
# Hack to import the pfs test utils from the devkit tests
import sys

sys.path.append("../promptflow-devkit/tests")
from sdk_pfs_test.utils import PFSOperations
temp_path = (
Path(PROMPTFLOW_ROOT)
.joinpath("..", "promptflow-devkit", "tests", "sdk_pfs_test")
.resolve()
.absolute()
.as_posix()
)
sys.path.append(temp_path)
# TODO: avoid doing this as utils is a widely used module name
from utils import PFSOperations

client = app.test_client()
return PFSOperations(client)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import datetime
import json
import platform
import sys
import typing
import uuid
Expand Down Expand Up @@ -354,6 +355,10 @@ def test_basic_search_line_runs(self, pf: PFClient) -> None:
line_runs = pf.traces._search_line_runs(expression=expr)
assert len(line_runs) == 1

@pytest.mark.skipif(
platform.system() == "Windows" and sys.version_info < (3, 9),
reason="Python 3.9+ is required on Windows to support json_extract",
)
def test_search_line_runs_with_tokens(self, pf: PFClient) -> None:
num_line_runs = 5
trace_ids = list()
Expand Down
4 changes: 2 additions & 2 deletions src/promptflow-devkit/tests/sdk_pfs_test/e2etests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
# ---------------------------------------------------------

import os
import platform
import subprocess
import sys

Expand Down Expand Up @@ -62,7 +61,8 @@ def test_start_service(self, capsys):
# previous pfs is killed
assert start_pfs.poll() is not None
python_dir = os.path.dirname(sys.executable)
executable_dir = os.path.join(python_dir, "Scripts") if platform.system() == "Windows" else python_dir
# python directory will be changed to Scripts directory after we switched to poetry in ci
executable_dir = python_dir
assert executable_dir in os.environ["PATH"].split(os.pathsep)
finally:
port = get_port_from_config()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import re
import sys

import pytest

from promptflow._core._errors import ToolExecutionError
from promptflow.contracts.run_info import Status
from promptflow.executor import FlowExecutor
from promptflow._core._errors import ToolExecutionError

from ..utils import (
get_yaml_file,
)
from ..utils import get_yaml_file

SAMPLE_FLOW = "web_classification_no_variants"
SAMPLE_EVAL_FLOW = "classification_accuracy_evaluation"
Expand All @@ -26,6 +27,7 @@
Traceback (most recent call last):
in wrapped
output = func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^
sync_fail.py", line 13, in raise_an_exception
raise Exception(f"In tool raise_an_exception: {s}") from e
Exception: In tool raise_an_exception: dummy_input
Expand All @@ -44,6 +46,7 @@
Traceback (most recent call last):
in wrapped
output = await func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
in raise_an_exception_async
raise Exception(f"In tool raise_an_exception_async: {s}") from e
Exception: In tool raise_an_exception_async: dummy_input
Expand All @@ -52,6 +55,11 @@
),
}

if sys.version_info < (3, 11):
# Python 3.11 on Mac has an extra line of ^^^^^ to point on the function raising the exception
for key in expected_stack_traces:
expected_stack_traces[key] = [line for line in expected_stack_traces[key] if re.match(r"^\s+\^+", line) is None]


@pytest.mark.e2etest
class TestExecutorFailures:
Expand Down Expand Up @@ -81,7 +89,9 @@ def test_executor_exec_node_fail(self, flow_folder, node_name, message):
# Make sure the stack trace is as expected
stacktrace = user_error_info["traceback"].split("\n")
expected_stack_trace = expected_stack_traces[flow_folder]
assert len(stacktrace) == len(expected_stack_trace)
if len(stacktrace) != len(expected_stack_trace):
# actually we should fail now; adding this to make sure we can see the difference
assert stacktrace == expected_stack_trace
for expected_item, actual_item in zip(expected_stack_trace, stacktrace):
assert expected_item in actual_item

Expand Down Expand Up @@ -112,7 +122,9 @@ def test_executor_exec_line_fail(self, flow_folder, failed_node_name, message):
# Make sure the stack trace is as expected
stacktrace = user_error_info["traceback"].split("\n")
expected_stack_trace = expected_stack_traces[flow_folder]
assert len(stacktrace) == len(expected_stack_trace)
if len(stacktrace) != len(expected_stack_trace):
# actually we should fail now; adding this to make sure we can see the difference
assert stacktrace == expected_stack_trace
for expected_item, actual_item in zip(expected_stack_trace, stacktrace):
assert expected_item in actual_item

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,15 @@


def test_get_token_with_sovereign_credential():
with (
patch('azure.ai.ml._azure_environments._get_default_cloud_name') as mock_cloud_name,
patch('azure.ai.ml._azure_environments._get_cloud') as mock_cloud,
patch('azure.identity.DefaultAzureCredential') as mock_credential,
):
mock_cloud_name.return_value = "AzureChinaCloud"
cloud = mock_cloud.return_value
cloud.get.return_value = "authority"
mock_token = "mocked_token"
mock_credential.return_value.get_token.return_value.token = mock_token
# use 3 with blocks to make it compatible with python 3.8
with patch("azure.ai.ml._azure_environments._get_default_cloud_name") as mock_cloud_name:
with patch("azure.ai.ml._azure_environments._get_cloud") as mock_cloud:
with patch("azure.identity.DefaultAzureCredential") as mock_credential:
mock_cloud_name.return_value = "AzureChinaCloud"
cloud = mock_cloud.return_value
cloud.get.return_value = "authority"
mock_token = "mocked_token"
mock_credential.return_value.get_token.return_value.token = mock_token

token_provider = AzureTokenProvider()
assert token_provider.get_token() == mock_token
token_provider = AzureTokenProvider()
assert token_provider.get_token() == mock_token
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@

from promptflow._core._errors import DuplicateToolMappingError
from promptflow._utils.tool_utils import (
RetrieveToolFuncResultError,
ListFunctionResponseError,
RetrieveToolFuncResultError,
RetrieveToolFuncResultValidationError,
_find_deprecated_tools,
append_workspace_triple_to_func_input_params,
Expand Down Expand Up @@ -376,7 +376,7 @@ def test_load_function_from_function_path_with_error(self, mock_module_with_list
(
ToolFuncCallScenario.REVERSE_GENERATED_BY,
"dummy_result",
f"ToolFuncCallScenario {ToolFuncCallScenario.REVERSE_GENERATED_BY} response must be a dict. "
f"ToolFuncCallScenario {ToolFuncCallScenario.REVERSE_GENERATED_BY.value} response must be a dict. "
f"dummy_result is not a dict.",
),
(
Expand Down

0 comments on commit 9f616ff

Please sign in to comment.