You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
### Motivation and Context
This PR slims down the number of integration tests running against AOAI
and OAI models - reducing time and resources spent running them.
Additional justification for slimming down these tests is to reduce the
chance of throttling from these endpoints (leads to unstable integration
tests).
The scenarios previously being covered are already handled using HF
models which are much less expensive to test against and do not run the
risk of throttling.
### Description
- added a top-level test `conftest.py` to handle kernel creation and
OpenAI model secret handling. I had originally intended `create_kernel`
to do more than just create a kernel, but additional setup wasn't
needed. I've left the pytest fixture though since other fixtures can
call it, and using it in tests can making importing Kernel from
semantic_kernel unncessary.
- added a completions `conftest.py` to set up completions tests. For
example, setup_hf_text_completion_function allows for testing
text2text_generation and text_generation models using the same test
file.
- Common test code is now in pytest fixtures instead of common methods.
- For a number of the completion tests, I have broken out the asserts to
individual tests instead of running one giant test. This makes it easier
to identify regressions in specific patterns around invoking skills.
- Added retry logic to conversationSummarySkill
- renamed tests to be more descriptive.
🎯 Main theme: Improving stability of Python integration tests
🔍 Description and title: Yes
📌 Type of PR: Enhancement
🧪 Relevant tests added: No
✨ Minimal and focused: Yes, the PR is focused on improving the stability of Python integration tests by adjusting the build system requirements and the parallelization of the tests.
🔒 Security concerns: No, the changes in this PR do not introduce any obvious security concerns.
PR Feedback
💡 General PR suggestions: The PR is generally well-structured and focused. However, it would be beneficial to add tests that verify the changes made in this PR. Also, consider adding comments in the code to explain why specific versions of dependencies are required (e.g., setuptools>=67.8.0).
🤖 Code suggestions:
relevant file: python/pyproject.toml suggestion content: Consider specifying the exact version of 'setuptools' that is known to work with your project, instead of '>=67.8.0'. This can help avoid potential issues with future versions of 'setuptools'. [important]
relevant file: .github/workflows/python-integration-tests.yml suggestion content: Consider adding a comment explaining why 'max-parallel' was increased from 1 to 3. This will help future contributors understand the reasoning behind this change. [medium]
How to use
Tag me in a comment '@CodiumAI-Agent' to ask for a new review after you update the PR.
You can also tag me and ask any question, for example '@CodiumAI-Agent is the PR ready for merge?'
pythonPull requests for the Python Semantic Kernel
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
shawncal
changed the title
Motivation and Context
This PR is intended to add stability to the python integration tests.
Description
See Actions runs:
Contribution Checklist
dotnet format