Skip to content

Fix test names for pytest #2358

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

Merged
merged 2 commits into from
Jun 2, 2025
Merged

Fix test names for pytest #2358

merged 2 commits into from
Jun 2, 2025

Conversation

justinchuby
Copy link
Collaborator

@justinchuby justinchuby commented Jun 2, 2025

With the latest version of pytest we get

onnxscript/rewriter/ort_fusions/cos_sin_cache_test.py::test_case_1 - Failed: Expected None, but test returned <onnxscript.rewriter.ort_fusions.models._rotary_embedding_models._TestCase1 object at 0x117c920d0>. Did you mean to use assert instead of return?

This is because the imported functions test_case_1 and test_case_2 are not really test cases but were treated as such by pytest. This PR hides them from the test module so they are not triggered.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR prevents pytest from discovering helper functions (test_case_1, test_case_2, etc.) as standalone test cases by switching from direct imports to module imports and updating all references accordingly.

  • Replaced direct imports of test-case constructors with module-level imports for _rotary_embedding_models and _smollm_1
  • Updated all usages to qualify test constructors with their module to hide them from pytest
Comments suppressed due to low confidence (2)

onnxscript/rewriter/ort_fusions/cos_sin_cache_test.py:54

  • [nitpick] Rename the variable test to something more descriptive (e.g., test_case or case) to avoid shadowing built-ins and clarify its purpose.
test = _rotary_embedding_models.partial_rotary_test_case()

onnxscript/rewriter/ort_fusions/cos_sin_cache_test.py:12

  • [nitpick] Consider aliasing these module imports to shorter names (e.g., _rotary_embedding_models as rem_models, _smollm_1 as smollm_models) to reduce verbosity and improve readability in the test code.
from onnxscript.rewriter.ort_fusions.models import _rotary_embedding_models, _smollm_1

Copy link

codecov bot commented Jun 2, 2025

❌ 4 Tests Failed:

Tests completed Failed Passed Skipped
15663 4 15659 1698
View the top 3 failed test(s) by shortest run time
onnxscript.backend.onnx_export_test.TestOnnxBackEnd::test_export2python_produces_correct_onnx_script_model_0795_test_range_float_type_positive_delta
Stack Traces | 0.004s run time
onnxscript\backend\onnx_export_test.py:137: in extract_functions
    mod = importlib.import_module(import_name)
C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\importlib\__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
E   ModuleNotFoundError: No module named 'tests.onnx_backend_test_code.test_range_float_type_positive_delta'

The above exception was the direct cause of the following exception:
.nox\test\lib\site-packages\parameterized\parameterized.py:620: in standalone_func
    return func(*(a + p.args), **p.kwargs, **kw)
onnxscript\backend\onnx_export_test.py:271: in test_export2python_produces_correct_onnx_script_model
    functions = extract_functions(backend_test.name, code, self.test_folder)
onnxscript\backend\onnx_export_test.py:139: in extract_functions
    raise AssertionError(
E   AssertionError: Unable to import 'tests.onnx_backend_test_code.test_range_float_type_positive_delta' (e=No module named 'tests.onnx_backend_test_code.test_range_float_type_positive_delta') (file: 'D:\\a\\onnxscript\\onnxscript\\tests\\onnx_backend_test_code\\test_range_float_type_positive_delta.py', absolute path: 'D:\\a\\onnxscript\\onnxscript\\tests\\onnx_backend_test_code\\test_range_float_type_positive_delta.py', current folder: D:\a\onnxscript\onnxscript
E   ---- CONTENT --
E   import numpy
E   from onnx import TensorProto
E   from onnx.helper import make_tensor
E   from onnxscript import script, external_tensor
E   from onnxscript.values import Opset
E   from onnxscript.onnx_types import FLOAT
E   from onnxscript.onnx_opset import opset11
E   
E   @script()
E   def bck_test_range_float_type_positive_delta(start: FLOAT, limit: FLOAT, delta: FLOAT) -> (FLOAT[2]):
E       output = opset11.Range(start, limit, delta)
E       return output
onnxscript.backend.onnx_export_test.TestOnnxBackEnd::test_export2python_produces_correct_onnx_script_model_0841_test_reduce_log_sum_desc_axes
Stack Traces | 0.005s run time
onnxscript\backend\onnx_export_test.py:137: in extract_functions
    mod = importlib.import_module(import_name)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
C:\hostedtoolcache\windows\Python\3.11.9\x64\Lib\importlib\__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
E   ModuleNotFoundError: No module named 'tests.onnx_backend_test_code.test_reduce_log_sum_desc_axes'

The above exception was the direct cause of the following exception:
.nox\test_ort_nightly\Lib\site-packages\parameterized\parameterized.py:620: in standalone_func
    return func(*(a + p.args), **p.kwargs, **kw)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
onnxscript\backend\onnx_export_test.py:271: in test_export2python_produces_correct_onnx_script_model
    functions = extract_functions(backend_test.name, code, self.test_folder)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
onnxscript\backend\onnx_export_test.py:139: in extract_functions
    raise AssertionError(
E   AssertionError: Unable to import 'tests.onnx_backend_test_code.test_reduce_log_sum_desc_axes' (e=No module named 'tests.onnx_backend_test_code.test_reduce_log_sum_desc_axes') (file: 'D:\\a\\onnxscript\\onnxscript\\tests\\onnx_backend_test_code\\test_reduce_log_sum_desc_axes.py', absolute path: 'D:\\a\\onnxscript\\onnxscript\\tests\\onnx_backend_test_code\\test_reduce_log_sum_desc_axes.py', current folder: D:\a\onnxscript\onnxscript
E   ---- CONTENT --
E   import numpy
E   from onnx import TensorProto
E   from onnx.helper import make_tensor
E   from onnxscript import script, external_tensor
E   from onnxscript.values import Opset
E   from onnxscript.onnx_types import FLOAT, INT64
E   from onnxscript.onnx_opset import opset18
E   
E   @script()
E   def bck_test_reduce_log_sum_desc_axes(data: FLOAT[3,4,5], axes: INT64[2]) -> (FLOAT[3]):
E       reduced = opset18.ReduceLogSum(data, axes, keepdims=0)
E       return reduced
onnxscript.backend.onnx_export_test.TestOnnxBackEnd::test_export2python_produces_correct_onnx_script_model_0864_test_reduce_log_sum_negative_axes_expanded
Stack Traces | 0.005s run time
onnxscript\backend\onnx_export_test.py:137: in extract_functions
    mod = importlib.import_module(import_name)
C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\importlib\__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
E   ModuleNotFoundError: No module named 'tests.onnx_backend_test_code.test_reduce_log_sum_negative_axes_expanded'

The above exception was the direct cause of the following exception:
.nox\test\lib\site-packages\parameterized\parameterized.py:620: in standalone_func
    return func(*(a + p.args), **p.kwargs, **kw)
onnxscript\backend\onnx_export_test.py:271: in test_export2python_produces_correct_onnx_script_model
    functions = extract_functions(backend_test.name, code, self.test_folder)
onnxscript\backend\onnx_export_test.py:139: in extract_functions
    raise AssertionError(
E   AssertionError: Unable to import 'tests.onnx_backend_test_code.test_reduce_log_sum_negative_axes_expanded' (e=No module named 'tests.onnx_backend_test_code.test_reduce_log_sum_negative_axes_expanded') (file: 'D:\\a\\onnxscript\\onnxscript\\tests\\onnx_backend_test_code\\test_reduce_log_sum_negative_axes_expanded.py', absolute path: 'D:\\a\\onnxscript\\onnxscript\\tests\\onnx_backend_test_code\\test_reduce_log_sum_negative_axes_expanded.py', current folder: D:\a\onnxscript\onnxscript
E   ---- CONTENT --
E   import numpy
E   from onnx import TensorProto
E   from onnx.helper import make_tensor
E   from onnxscript import script, external_tensor
E   from onnxscript.values import Opset
E   from onnxscript.onnx_types import FLOAT, INT64
E   from onnxscript.onnx_opset import opset18
E   
E   @script()
E   def bck_test_reduce_log_sum_negative_axes_expanded(data: FLOAT[3,4,5], axes: INT64[1]) -> (FLOAT[3,1,5]):
E       ReduceLogSum_test_reduce_log_sum_negative_axes_expanded_function_reduced_sum = opset18.ReduceSum(data, axes, keepdims=1)
E       reduced = opset18.Log(ReduceLogSum_test_reduce_log_sum_negative_axes_expanded_function_reduced_sum)
E       return reduced

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@justinchuby justinchuby changed the title Fix pytest for TestCosSinCacheTransform Fix test names for pytest Jun 2, 2025
Copy link
Contributor

@titaiwangms titaiwangms left a comment

Choose a reason for hiding this comment

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

Looks like a bad naming. Maybe we should consider rename it.

@github-project-automation github-project-automation bot moved this from Todo to Done in ONNX Script Review Board Jun 2, 2025
@justinchuby
Copy link
Collaborator Author

Looks like a bad naming. Maybe we should consider rename it.

Agreed

@justinchuby justinchuby merged commit 4e526f7 into main Jun 2, 2025
26 of 30 checks passed
@justinchuby justinchuby deleted the justinchu/fix-pytest branch June 2, 2025 23:35
bmehta001 pushed a commit to bmehta001/onnxscript that referenced this pull request Jun 5, 2025
With the latest version of pytest we get 
`onnxscript/rewriter/ort_fusions/cos_sin_cache_test.py::test_case_1 -
Failed: Expected None, but test returned
<onnxscript.rewriter.ort_fusions.models._rotary_embedding_models._TestCase1
object at 0x117c920d0>. Did you mean to use `assert` instead of
`return`?`

This is because the imported functions `test_case_1` and `test_case_2`
are not really test cases but were treated as such by pytest. This PR
hides them from the test module so they are not triggered.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

2 participants