From e546dbd6b8e3df66a57d99d31441f20f24fc7641 Mon Sep 17 00:00:00 2001 From: yzlin Date: Thu, 3 Aug 2023 16:00:28 +0800 Subject: [PATCH] use profile as sender & fix some bugs --- metagpt/actions/debug_error.py | 6 ++--- metagpt/actions/run_code.py | 17 ++++++------ metagpt/actions/write_test.py | 4 +-- metagpt/roles/qa_engineer.py | 32 ++++++++++++++--------- tests/metagpt/actions/test_debug_error.py | 5 ++-- 5 files changed, 35 insertions(+), 29 deletions(-) diff --git a/metagpt/actions/debug_error.py b/metagpt/actions/debug_error.py index 437f7b9d4..d69a22dba 100644 --- a/metagpt/actions/debug_error.py +++ b/metagpt/actions/debug_error.py @@ -14,15 +14,15 @@ from metagpt.utils.common import CodeParser PROMPT_TEMPLATE = """ NOTICE 1. Role: You are a Development Engineer or QA engineer; -2. Task: You received this message from another Development Engineer or QA engineer who run or test your code. +2. Task: You received this message from another Development Engineer or QA engineer who ran or tested your code. Based on the message, first, figure out your own role, i.e. Engineer or QaEngineer, then rewrite the development code or the test code based on your role, the error, and the summary, such that all bugs are fixed and the code performs well. -Attention: Use '##' to split sections, not '#', and '## ' SHOULD WRITE BEFORE the test case or script and triple quote. +Attention: Use '##' to split sections, not '#', and '## ' SHOULD WRITE BEFORE the test case or script and triple quotes. The message is as follows: {context} --- Now you should start rewriting the code: -## file name of the code to rewrite: Write code with triple quoto. Do your best to implement THIS ONLY ONE FILE. +## file name of the code to rewrite: Write code with triple quoto. Do your best to implement THIS IN ONLY ONE FILE. """ class DebugError(Action): def __init__(self, name="DebugError", context=None, llm=None): diff --git a/metagpt/actions/run_code.py b/metagpt/actions/run_code.py index 1334c9127..1bc5cc13a 100644 --- a/metagpt/actions/run_code.py +++ b/metagpt/actions/run_code.py @@ -8,6 +8,7 @@ import traceback import os import subprocess +from typing import List, Tuple from metagpt.logs import logger from metagpt.actions.action import Action @@ -59,18 +60,18 @@ class RunCode(Action): super().__init__(name, context, llm) @classmethod - async def run_text(cls, code): + async def run_text(cls, code) -> Tuple[str, str]: try: # We will document_store the result in this dictionary namespace = {} exec(code, namespace) - return namespace.get('result', None), "" + return namespace.get('result', ""), "" except Exception: # If there is an error in the code, return the error message return "", traceback.format_exc() @classmethod - async def run_script(cls, working_directory, additional_python_paths=[], command=[]): + async def run_script(cls, working_directory, additional_python_paths=[], command=[]) -> Tuple[str, str]: working_directory = str(working_directory) additional_python_paths = [str(path) for path in additional_python_paths] @@ -93,16 +94,16 @@ class RunCode(Action): process.kill() # Kill the process if it times out stdout, stderr = process.communicate() return stdout.decode('utf-8'), stderr.decode('utf-8') - + async def run( self, code, mode="script", code_file_name="", test_code="", test_file_name="", command=[], **kwargs - ): + ) -> str: logger.info(f"Running {' '.join(command)}") if mode == "script": outs, errs = await self.run_script(command=command, **kwargs) elif mode == "text": outs, errs = await self.run_text(code=code) - + logger.info(f"{outs=}") logger.info(f"{errs=}") @@ -111,9 +112,9 @@ class RunCode(Action): test_code=test_code, test_file_name=test_file_name, command=" ".join(command), outs=outs[:500], # outs might be long but they are not important, truncate them to avoid token overflow - errs=errs + errs=errs[:10000] # truncate errors to avoid token overflow ) - + prompt = PROMPT_TEMPLATE.format(context=context) rsp = await self._aask(prompt) diff --git a/metagpt/actions/write_test.py b/metagpt/actions/write_test.py index 511399f7d..e1c1571c3 100644 --- a/metagpt/actions/write_test.py +++ b/metagpt/actions/write_test.py @@ -15,10 +15,10 @@ NOTICE 2. Requirement: Based on the context, develop a comprehensive test suite that adequately covers all relevant aspects of the code file under review. Your test suite will be part of the overall project QA, so please develop complete, robust, and reusable test cases. 3. Attention1: Use '##' to split sections, not '#', and '## ' SHOULD WRITE BEFORE the test case or script. 4. Attention2: If there are any settings in your tests, ALWAYS SET A DEFAULT VALUE, ALWAYS USE STRONG TYPE AND EXPLICIT VARIABLE. -5. Attention3: YOU MUST FOLLOW "Data structures and interface definitions". DO NOT CHANGE ANY DESIGN. Make sure your tests respect the existing design and ensure their validity. +5. Attention3: YOU MUST FOLLOW "Data structures and interface definitions". DO NOT CHANGE ANY DESIGN. Make sure your tests respect the existing design and ensure its validity. 6. Think before writing: What should be tested and validated in this document? What edge cases could exist? What might fail? 7. CAREFULLY CHECK THAT YOU DON'T MISS ANY NECESSARY TEST CASES/SCRIPTS IN THIS FILE. -Attention: Use '##' to split sections, not '#', and '## ' SHOULD WRITE BEFORE the test case or script and triple quote. +Attention: Use '##' to split sections, not '#', and '## ' SHOULD WRITE BEFORE the test case or script and triple quotes. ----- ## Given the following code, please write appropriate test cases using Python's unittest framework to verify the correctness and robustness of this code: ```python diff --git a/metagpt/roles/qa_engineer.py b/metagpt/roles/qa_engineer.py index 597249dff..303b775ac 100644 --- a/metagpt/roles/qa_engineer.py +++ b/metagpt/roles/qa_engineer.py @@ -5,6 +5,7 @@ @Author : alexanderwu @File : qa_engineer.py """ +import os import re from pathlib import Path from typing import Type @@ -19,7 +20,7 @@ from metagpt.utils.common import CodeParser, parse_recipient from metagpt.utils.special_tokens import MSG_SEP, FILENAME_CODE_SEP class QaEngineer(Role): - def __init__(self, name="Edward", profile="QA Engineer", + def __init__(self, name="Edward", profile="QaEngineer", goal="Write comprehensive and robust tests to ensure codes will work as expected without bugs", constraints="The test code you write should conform to code standard like PEP8, be modular, easy to read and maintain"): super().__init__(name, profile, goal, constraints) @@ -86,7 +87,7 @@ class QaEngineer(Role): } msg = Message( content=str(file_info), role=self.profile, cause_by=WriteTest, - sent_from="QaEngineer", send_to="QaEngineer" + sent_from=self.profile, send_to=self.profile ) self._publish_message(msg) @@ -94,14 +95,19 @@ class QaEngineer(Role): async def _run_code(self, msg): file_info = eval(msg.content) - code_to_test = open(file_info["file_path"], "r").read() - test_code = open(file_info["test_file_path"], "r").read() + development_file_path = file_info["file_path"] + test_file_path = file_info["test_file_path"] + if not os.path.exists(development_file_path) or not os.path.exists(test_file_path): + return + + development_code = open(development_file_path, "r").read() + test_code = open(test_file_path, "r").read() proj_dir = self.get_workspace() development_code_dir = self.get_workspace(return_proj_dir=False) result_msg = await RunCode().run( mode="script", - code=code_to_test, + code=development_code, code_file_name=file_info["file_name"], test_code=test_code, test_file_name=file_info["test_file_name"], @@ -115,7 +121,7 @@ class QaEngineer(Role): content = str(file_info) + FILENAME_CODE_SEP + result_msg msg = Message( content=content, role=self.profile, cause_by=RunCode, - sent_from="QaEngineer", send_to=recipient + sent_from=self.profile, send_to=recipient ) self._publish_message(msg) @@ -125,20 +131,20 @@ class QaEngineer(Role): if file_name: self.write_file(file_name, code) recipient = msg.sent_from # send back to the one who ran the code for another run, might be one's self - msg = Message(content=file_info, role=self.profile, cause_by=DebugError, sent_from="QaEngineer", send_to=recipient) + msg = Message(content=file_info, role=self.profile, cause_by=DebugError, sent_from=self.profile, send_to=recipient) self._publish_message(msg) async def _observe(self) -> int: await super()._observe() self._rc.news = [msg for msg in self._rc.news \ - if msg.send_to == "QaEngineer"] # only relevant msgs count as observed news + if msg.send_to == self.profile] # only relevant msgs count as observed news return len(self._rc.news) async def _act(self) -> Message: if self.test_round > self.test_round_allowed: result_msg = Message( content=f"Exceeding {self.test_round_allowed} rounds of tests, skip (writing code counts as a round, too)", - role=self.profile, cause_by=WriteTest, sent_from="QaEngineer", send_to="" + role=self.profile, cause_by=WriteTest, sent_from=self.profile, send_to="" ) return result_msg @@ -147,16 +153,16 @@ class QaEngineer(Role): # might potentially be moved to _think, that is, let the agent decides for itself if msg.cause_by == WriteCode: # engineer wrote a code, time to write a test for it - result_msg = await self._write_test(msg) + await self._write_test(msg) elif msg.cause_by in [WriteTest, DebugError]: # I wrote or debugged my test code, time to run it - result_msg = await self._run_code(msg) + await self._run_code(msg) elif msg.cause_by == RunCode: # I ran my test code, time to fix bugs, if any - result_msg = await self._debug_error(msg) + await self._debug_error(msg) self.test_round += 1 result_msg = Message( content=f"Round {self.test_round} of tests done", - role=self.profile, cause_by=WriteTest, sent_from="QaEngineer", send_to="" + role=self.profile, cause_by=WriteTest, sent_from=self.profile, send_to="" ) return result_msg diff --git a/tests/metagpt/actions/test_debug_error.py b/tests/metagpt/actions/test_debug_error.py index 23eecb2f8..555c84e4e 100644 --- a/tests/metagpt/actions/test_debug_error.py +++ b/tests/metagpt/actions/test_debug_error.py @@ -146,11 +146,10 @@ Engineer @pytest.mark.asyncio async def test_debug_error(): - code = "def add(a, b):\n return a - b" - error = "AssertionError: Expected add(1, 1) to equal 2 but got 0" debug_error = DebugError("debug_error") file_name, rewritten_code = await debug_error.run(context=EXAMPLE_MSG_CONTENT) - assert "class TestPlayer" in rewritten_code + assert "class Player" in rewritten_code # rewrite the same class + assert "while self.score > 21" in rewritten_code # a key logic to rewrite to (original one is "if self.score > 12")