Fix "update cassettes" step #4591

waynehamadi · 2023-06-05T23:50:38Z

Background

https://github.com/Significant-Gravitas/Auto-GPT/actions/runs/5181119242/jobs/9336197276?pr=4581

Update cassettes steps don't work. this is the fix.

Changes

fix the update cassettes step
removed some code and simplified the ci.yml

Documentation

Test Plan

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 · 2023-06-05T23:50:41Z

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

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
docs	⬜️ Ignored (Inspect)			Jun 6, 2023 2:09pm

.github/workflows/ci.yml

waynehamadi · 2023-06-05T23:51:36Z

.github/workflows/ci.yml

-              git branch -D $cassette_branch
-            fi
-            echo "cassette_branch=$(git branch --show-current)" >> $GITHUB_OUTPUT
+            git merge --no-commit --strategy-option ours origin/${{ github.event.pull_request.base.ref }}


merge with --ours strategy: we pick what can be picked from master, otherwise we just use the cache built previously.

Looks like a nice way to do this.

Note: it picks from the base branch, which doesn't necessarily have to be master. What happens when a PR is created against stable?

By the way, why the --no-commit? I see it was there before but the reason isn't obvious to me.

.github/workflows/ci.yml

waynehamadi · 2023-06-05T23:57:19Z

.github/workflows/ci.yml

-            curl -X DELETE \
-            -H "Authorization: Bearer $TOKEN" \
-            -H "Accept: application/vnd.github.v3+json" \
-            https://api.github.com/repos/$REPO/issues/$PR_NUMBER/labels/behaviour%20change


why ? because I realized this doesn't work properly, and it's very hard to make it work properly. Let me explain:

you run your tests

you get new cassettes and it says that you changed the behaviour

you're surprised because it wasn't intended.

you change your code

you push again. Unfortunately at that time we still have your old cassettes, so we can't really tell if you went back to normal. If we pick the cassettes of master, then the tests are way slower.

I will think about it more. It's already pretty good to be able to detect behaviour change, unfortunately, we can't detect if the behaviour returns to normal compared to master.

If we can detect which parts of the cassettes are actually being used, we can reset cassettes when their last entry is not used during a test run. (assuming new requests are always added to the end of a cassette)

yeah that's a good trick

Auto-GPT-Bot · 2023-06-05T23:57:47Z

You changed AutoGPT's behaviour. The cassettes have been updated and will be merged to the submodule when this Pull Request gets merged.

codecov · 2023-06-05T23:59:54Z

Codecov Report

Patch and project coverage have no change.

Comparison is base (60ac0c4) 69.58% compared to head (8803f5d) 69.58%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4591   +/-   ##
=======================================
  Coverage   69.58%   69.58%           
=======================================
  Files          72       72           
  Lines        3551     3551           
  Branches      569      569           
=======================================
  Hits         2471     2471           
  Misses        889      889           
  Partials      191      191

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

Pwuts

Nice clean-up!

Pwuts · 2023-06-06T12:56:10Z

.github/workflows/ci.yml

-              git branch -D $cassette_branch
-            fi
-            echo "cassette_branch=$(git branch --show-current)" >> $GITHUB_OUTPUT
+            git merge --no-commit --strategy-option ours origin/${{ github.event.pull_request.base.ref }}


Looks like a nice way to do this.

Note: it picks from the base branch, which doesn't necessarily have to be master. What happens when a PR is created against stable?

Pwuts · 2023-06-06T13:19:56Z

.github/workflows/ci.yml

-              git branch -D $cassette_branch
-            fi
-            echo "cassette_branch=$(git branch --show-current)" >> $GITHUB_OUTPUT
+            git merge --no-commit --strategy-option ours origin/${{ github.event.pull_request.base.ref }}


By the way, why the --no-commit? I see it was there before but the reason isn't obvious to me.

Pwuts · 2023-06-06T13:37:28Z

.github/workflows/ci.yml

-            curl -X DELETE \
-            -H "Authorization: Bearer $TOKEN" \
-            -H "Accept: application/vnd.github.v3+json" \
-            https://api.github.com/repos/$REPO/issues/$PR_NUMBER/labels/behaviour%20change


If we can detect which parts of the cassettes are actually being used, we can reset cassettes when their last entry is not used during a test run. (assuming new requests are always added to the end of a cassette)

.github/workflows/ci.yml

Pwuts

I'll merge this, with the sidenote that there is currently no solid workflow for PRs to stable as CI does not run on push to stable. This means PRs with updates to cassettes or behavior must always be made to master, or manual work is required to make sure the PR includes an update to the submodule reference.

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)

github-actions bot added the size/l label Jun 5, 2023