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

Rearrange tests & fix CI #4596

Merged
merged 11 commits into from
Jun 6, 2023
Merged

Rearrange tests & fix CI #4596

merged 11 commits into from
Jun 6, 2023

Conversation

Pwuts
Copy link
Member

@Pwuts Pwuts commented Jun 6, 2023

Background

  • Not all tests are in the right location (tests/ vs tests/(unit|integration)/)
  • Challenges are run as part of the integration tests, while it can be useful to run only integration tests and not the challenges
  • We have a bunch of bogus unit tests which contribute more headache than valuable test coverage
  • Docker CI will succeed even if some of the tests fail

Changes

  • Rearrange all tests into tests/unit, tests/integration and tests/challenges
    • Some tests also moved from tests/integration to tests/unit
  • Clean up bogus unit tests
  • Reduce Docker CI to only run unit+integration tests
  • Fix Docker CI to actually fail when the tests fail

Other changes:

Documentation

References to tests/integration/challenges or tests.integration.challenges in docs were also updated.

Test Plan

Run CI and check result + coverage

PR Quality Checklist

  • My pull request is atomic and focuses on a single change.
  • I have thoroughly tested my changes with multiple different prompts.
  • I have considered potential risks and mitigations for my changes.
  • I have documented my changes clearly and comprehensively.
  • I have not snuck in any "extra" small tweaks changes.
  • I have run the following commands against my code to ensure it passes our linters:
    black .
    isort .
    mypy
    autoflake --remove-all-unused-imports --recursive --ignore-init-module-imports autogpt tests --in-place

@vercel
Copy link

vercel bot commented Jun 6, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
docs ⬜️ Ignored (Inspect) Visit Preview Jun 6, 2023 4:49pm

@github-actions github-actions bot added the size/m label Jun 6, 2023
@vercel vercel bot temporarily deployed to Preview June 6, 2023 15:26 Inactive
@Pwuts Pwuts self-assigned this Jun 6, 2023
@github-actions github-actions bot added size/l and removed size/m labels Jun 6, 2023
@vercel vercel bot temporarily deployed to Preview June 6, 2023 15:36 Inactive
@github-actions github-actions bot added size/xl and removed size/l labels Jun 6, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2023

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size

@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2023

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size

@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2023

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size

@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2023

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2023

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size

@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2023

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2023

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size

@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2023

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size

@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.20 ⚠️

Comparison is base (ee6b97e) 69.58% compared to head (57eb545) 69.38%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4596      +/-   ##
==========================================
- Coverage   69.58%   69.38%   -0.20%     
==========================================
  Files          72       72              
  Lines        3551     3551              
  Branches      569      569              
==========================================
- Hits         2471     2464       -7     
- Misses        889      895       +6     
- Partials      191      192       +1     
Impacted Files Coverage Δ
autogpt/app.py 50.00% <0.00%> (+8.88%) ⬆️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2023

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size

@Pwuts Pwuts marked this pull request as ready for review June 6, 2023 16:50
@Pwuts
Copy link
Member Author

Pwuts commented Jun 6, 2023

Someone will have to take a look at test_json_parser.py and _test_json_parser.py, determine what we actually want fix_and_parse_json() to be capable of, and consolidate.

Copy link
Member

@ntindle ntindle left a comment

Choose a reason for hiding this comment

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

Make sure the mypy config is updated

@ntindle
Copy link
Member

ntindle commented Jun 6, 2023

Someone will have to take a look at test_json_parser.py and _test_json_parser.py, determine what we actually want fix_and_parse_json() to be capable of, and consolidate.

We should make a new issue for this rather than holding this one up

@waynehamadi waynehamadi mentioned this pull request Jun 6, 2023
1 task
@waynehamadi waynehamadi merged commit dafbd11 into master Jun 6, 2023
12 of 13 checks passed
@waynehamadi waynehamadi deleted the dx/rearrange+fix-tests branch June 6, 2023 17:48
@waynehamadi
Copy link
Contributor

Someone will have to take a look at test_json_parser.py and _test_json_parser.py, determine what we actually want fix_and_parse_json() to be capable of, and consolidate.

We should make a new issue for this rather than holding this one up

#4597

lc0rp added a commit that referenced this pull request Jun 19, 2023
Co-authored-by: Reinier van der Leer <github@pwuts.nl>
Co-authored-by: Nicholas Tindle <nick@ntindle.com>
Co-authored-by: Nicholas Tindle <nicktindle@outlook.com>
Co-authored-by: k-boikov <64261260+k-boikov@users.noreply.github.com>
Co-authored-by: merwanehamadi <merwanehamadi@gmail.com>
Co-authored-by: Merwane Hamadi <merwanehamadi@gmail.com>
Co-authored-by: Richard Beales <rich@richbeales.net>
Co-authored-by: Luke K <2609441+lc0rp@users.noreply.github.com>
Co-authored-by: Luke K (pr-0f3t) <2609441+lc0rp@users.noreply.github.com>
Co-authored-by: Erik Peterson <e@eriklp.com>
Co-authored-by: Auto-GPT-Bot <github-bot@agpt.co>
Co-authored-by: Benny van der Lans <49377421+bfalans@users.noreply.github.com>
Co-authored-by: Jan <jan-github@phobia.de>
Co-authored-by: Robin Richtsfeld <robin.richtsfeld@gmail.com>
Co-authored-by: Marc Bornträger <marc.borntraeger@gmail.com>
Co-authored-by: Stefan Ayala <stefanayala3266@gmail.com>
Co-authored-by: javableu <45064273+javableu@users.noreply.github.com>
Co-authored-by: DGdev91 <DGdev91@users.noreply.github.com>
Co-authored-by: Kinance <kinance@gmail.com>
Co-authored-by: digger yu <digger-yu@outlook.com>
Co-authored-by: David <scenaristeur@gmail.com>
Co-authored-by: gravelBridge <john.tian31@gmail.com>
Fix Python CI "update cassettes" step (#4591)
fix CI (#4596)
Fix inverted logic for deny_command (#4563)
fix current_score.json generation (#4601)
Fix duckduckgo rate limiting (#4592)
Fix debug code challenge (#4632)
Fix issues with information retrieval challenge a (#4622)
fix issues with env configuration and .env.template (#4630)
Fix prompt issue causing 'No Command' issues and challenge to fail (#4623)
Fix benchmark logs (#4653)
Fix typo in docs/setup.md (#4613)
Fix run.sh shebang (#4561)
Fix autogpt docker image not working because missing prompt_settings (#4680)
Fix execute_command coming from plugins (#4730)
jordankanter pushed a commit to jordankanter/Auto-GPT that referenced this pull request Nov 12, 2023
* Rearrange tests into unit/integration/challenge categories

* Fix linting + `tests.challenges` imports

* Fix obscured duplicate test in test_url_validation.py

* Move VCR conftest to tests.vcr

* Specify tests to run & their order (unit -> integration -> challenges) in CI

* Fail Docker CI when tests fail

* Fix import & linting errors in tests

* Fix `get_text_summary`

* Fix linting errors

* Clean up pytest args in CI

* Remove bogus tests from GoCodeo
jordankanter pushed a commit to jordankanter/Auto-GPT that referenced this pull request Nov 12, 2023
* Rearrange tests into unit/integration/challenge categories

* Fix linting + `tests.challenges` imports

* Fix obscured duplicate test in test_url_validation.py

* Move VCR conftest to tests.vcr

* Specify tests to run & their order (unit -> integration -> challenges) in CI

* Fail Docker CI when tests fail

* Fix import & linting errors in tests

* Fix `get_text_summary`

* Fix linting errors

* Clean up pytest args in CI

* Remove bogus tests from GoCodeo
jordankanter pushed a commit to jordankanter/Auto-GPT that referenced this pull request Nov 12, 2023
* Rearrange tests into unit/integration/challenge categories

* Fix linting + `tests.challenges` imports

* Fix obscured duplicate test in test_url_validation.py

* Move VCR conftest to tests.vcr

* Specify tests to run & their order (unit -> integration -> challenges) in CI

* Fail Docker CI when tests fail

* Fix import & linting errors in tests

* Fix `get_text_summary`

* Fix linting errors

* Clean up pytest args in CI

* Remove bogus tests from GoCodeo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants