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

make integration test a little looser #1406

Closed
wants to merge 4 commits into from
Closed

make integration test a little looser #1406

wants to merge 4 commits into from

Conversation

rbren
Copy link
Collaborator

@rbren rbren commented Apr 27, 2024

@li-boxuan how bad an idea is this?

The idea is that instead of string-matching the prompts, we just iterate through the responses, one by one

@li-boxuan
Copy link
Collaborator

The idea is that instead of string-matching the prompts, we just iterate through the responses, one by one

I thought so at the beginning as well. The upside is clear - CI won't fail if one just modifies the prompt by a little bit.

The downsides are debatable though:

  1. One can break the agent, say, by always sending nonsense to LLM, and still get all these tests pass. A malicious attempt will mostly likely be caught by reviewers, but a true bug may not.
  2. I feel like keeping the prompt files as test artifacts is a good way for developers and even users to learn what are being sent to the LLM.

@rbren
Copy link
Collaborator Author

rbren commented May 5, 2024

Going to close this--seems like there hasn't been as much integration test pain lately. But we should definitely work on a better way to generate the prompt files

@rbren rbren closed this May 5, 2024
@li-boxuan
Copy link
Collaborator

That’s on my radar! Planning to work on that next week.

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.

None yet

3 participants