Skip to content

Comments

Disentangle tests #1076#1104

Merged
ATheorell merged 8 commits intomainfrom
disentangle_tests_1076
Apr 5, 2024
Merged

Disentangle tests #1076#1104
ATheorell merged 8 commits intomainfrom
disentangle_tests_1076

Conversation

@ATheorell
Copy link
Collaborator

As stated in #1076 the tests are way too tangled. In particular, usage of cached chatgpt responses in the tests have led to that most small changes have lead to multiple failing tests. This has been particularly challenging for new contributors.

In this PR:

  • No more caching in tests.
  • test_main.py no longer tests any workflow execution, only setup.
  • A single test in test_install.py runs the installed gpte executable WITH api key, no other test does. This test is marked with "requires_key" after suggestion by @ErikBjare. No other tests require api key.
  • Testing of running full improve workflows from the main currently missing, should be satisfied with specific unit tests of file_selector.py

@ATheorell
Copy link
Collaborator Author

Tox fails with exception that it cannot find valid OPENAI_API_KEY, though I've tried adding it in the ci.yaml

name: Run tox
env:
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
run: tox

There is a secret called OPENAI_API_KEY

Do you have an idea what could be going wrong here @ErikBjare ? I saw that you have a rather advanced setup including a key in https://github.com/ErikBjare/gptme/blob/master/.github/workflows/bot.yml

@ErikBjare
Copy link
Collaborator

ErikBjare commented Apr 4, 2024

@ATheorell I misunderstood, I'm not sure why it isn't working as it currently looks.

I figured it must be a quirk of tox, and I found this: https://tox.wiki/en/latest/config.html#passenv

So we need to set passenv=OPENAI_API_KEY (or whatever the syntax is), for tox to pass along the environment variable.

@ErikBjare
Copy link
Collaborator

ErikBjare commented Apr 5, 2024

@ATheorell Took the liberty, we'll see if it works...

Edit: looks like that did it!

@codecov
Copy link

codecov bot commented Apr 5, 2024

Codecov Report

Attention: Patch coverage is 47.36842% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 75.79%. Comparing base (1ad0892) to head (bdb4dab).

Files Patch % Lines
gpt_engineer/applications/cli/main.py 41.17% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1104      +/-   ##
==========================================
- Coverage   84.33%   75.79%   -8.54%     
==========================================
  Files          27       27              
  Lines        1564     1566       +2     
==========================================
- Hits         1319     1187     -132     
- Misses        245      379     +134     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ATheorell ATheorell merged commit df593e1 into main Apr 5, 2024
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.

2 participants