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

Streamline Testing with pytest-cov and pytest Defaults #2490

Merged
merged 39 commits into from
Apr 29, 2024

Conversation

WaelKarkoub
Copy link
Contributor

@WaelKarkoub WaelKarkoub commented Apr 23, 2024

Why are these changes needed?

This PR aims to simplify the testing process and reduce redundancy in the GitHub workflows.

This PR does the following:

  • Migrates to pytest-cov: Instead of relying on the coverage package, this PR adds pytest-cov plugin. pytest-cov works well with pytest-xdist, allowing us to run tests concurrently if the CI environment supports it.
  • Unifies test execution: The workflows have been updated to use the pytest command directly, replacing the previous coverage -a -m invocation. This unifies the testing process and simplifies the workflows.

Update 1:

Copy pasting from discord:

There's seems to be a silent failure in master:

  • with coverage the github action step passes even though the tests are failing.
  • with pytest-cov, the github action step fails correctly

The environment variable AUTOGEN_USE_DOCKER was not being used properly in these tests, so I added some fixes:

  • added a fix in test_commandline_code_executor.py,
  • added a fix in test/test_code_utils.py

Update 2:

  • There is a bug in Disc cache that doesn't handle windows path correctly, so I added a fix there as well.
  • This also led to a CI a bug PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpcq4z30pm\\42\\cache.db', for windows-2019 and python 3.12 for image gen cache test. I added a skip for that condition till I figure out the env.

Update 3:

  • Dropped openai version to openai>=1.3
  • Mypy ignores openai_utils.

Related issue number

Checks

@WaelKarkoub
Copy link
Contributor Author

@sonichi we're almost there! test/oai/test_client_stream.py::test_chat_tools_stream is a flaky test (even for me), and it doesn't test what it's supposed to be testing. I added a fix for it as well

@sonichi
Copy link
Contributor

sonichi commented Apr 28, 2024

@WaelKarkoub Thanks. There is some notebook test failure which I will fix.
@ekzhu @thinkall this is one reason we don't want to use a single config in the notebook.

@sonichi sonichi mentioned this pull request Apr 28, 2024
3 tasks
Merged via the queue into main with commit a917121 Apr 29, 2024
84 of 85 checks passed
@sonichi sonichi deleted the aggregate-pytest-settings branch April 29, 2024 00:33
jayralencar pushed a commit to jayralencar/autogen that referenced this pull request May 28, 2024
…#2490)

* done

* update docs

* try fix

* update workflows

* undo minor fix

* resolve comments

* adds back pytest-asyncio

* minor fix

* add branch coverage

* restore pip install e.

* test with coverage

* fix mypy

* fix coverage + docker + windows combo

* fix bash command

* formatter

* formatter

* one last fix

* I lied, last fix

* fix

* fix retrieve chat test

* fix windows paths

* change cache seed

* down grade openai version

* fix openai mypy

* better error type

* fix image gen cache test

* fix

* experimenting

* fix lmm

* skip cosmos test

* remove cosmos db

* unused imports

* handle more cosmosdb skips

* fix flaky test
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.

4 participants