-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fixed/improved error parsing from API messages. #3922
Merged
Merged
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
e3dbb82
Fixed/improved error parsing from API messages.
adbridge ca6bfe0
Review comments: Add a child logger, close json file after reading, m…
adbridge 9649d36
Added running total for target being compiled.
adbridge 828b7ac
Add an ignore list so that sets of test, target can be excluded from the
adbridge File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ | |
"name" : "test_compile_mbed_dev", | ||
"lib" : "mbed-dev" | ||
} | ||
] | ||
], | ||
"target_list" : [] | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,8 @@ | |
# "name" : "test_compile_mbed_dev", | ||
# "lib" : "mbed-dev" | ||
# } | ||
# ] | ||
# ], | ||
# "target_list" : [] | ||
#} | ||
# | ||
# The mbed_repo_path field should be changed to point to where your local | ||
|
@@ -41,6 +42,10 @@ | |
# "test_compile_mbed_lib" and "test_compile_mbed_dev" | ||
# The lib field in each says which type of mbed 2 library the app contains. | ||
# These test apps MUST be available as repos in the user's online Mercurial area. | ||
# The target_list allows the user to override the set of targets/platforms used | ||
# for the compilation. | ||
# E.g to just compile for 2 targets, K64F and K22F : | ||
# "target_list" : ["K64F", "K22F"] | ||
# | ||
# Run the script from the mbed-os directory as follows: | ||
# > python tools/check_release.py | ||
|
@@ -51,8 +56,8 @@ | |
# The lib files within the test apps are then updated to the corresponding version in | ||
# the associated lib itself. The test apps are then committed and pushed back to the users | ||
# fork. | ||
# The test apps will then be compiled for all supported targets and a % result output at the | ||
# end. | ||
# The test apps will then be compiled for all supported targets and a % result output at | ||
# the end. | ||
# | ||
# Uses the online compiler API at https://mbed.org/handbook/Compile-API | ||
# Based on the example from https://mbed.org/teams/mbed/code/mbed-API-helper/ | ||
|
@@ -76,21 +81,39 @@ | |
|
||
def get_compilation_failure(messages): | ||
""" Reads the json formatted 'messages' and checks for compilation errors. | ||
If there is a genuine compilation error then there should be a new message containing | ||
a severity field = Error and an accompanying message with the compile error text. | ||
Any other combination is considered an internal compile engine failure | ||
If there is a genuine compilation error then there should be a new | ||
message containing a severity field = Error and an accompanying message | ||
with the compile error text. Any other combination is considered an | ||
internal compile engine failure | ||
Args: | ||
messages - json formatted text returned by the online compiler API. | ||
|
||
Returns: | ||
Either a string containing a compilation error or "Internal" to indicate an error with | ||
the online IDE API itself. | ||
Either "Error" or "Internal" to indicate an actual compilation error or an | ||
internal IDE API fault. | ||
|
||
""" | ||
for m in messages: | ||
if 'severity' in m and 'message' in m: | ||
if m['severity'] == 'error': | ||
return m['message'] | ||
# Get message text if it exists | ||
try: | ||
message = m['message'] | ||
message = message + "\n" | ||
except KeyError: | ||
# Skip this message as it has no 'message' field | ||
continue | ||
|
||
# Get type of message text | ||
try: | ||
msg_type = m['type'] | ||
except KeyError: | ||
# Skip this message as it has no 'type' field | ||
continue | ||
|
||
if msg_type == 'error' or msg_type == 'tool_error': | ||
rel_log.error(message) | ||
return "Error" | ||
else: | ||
rel_log.debug(message) | ||
|
||
return "Internal" | ||
|
||
|
@@ -111,57 +134,61 @@ def invoke_api(payload, url, auth, polls, begin="start/"): | |
""" | ||
|
||
# send task to api | ||
logging.debug(url + begin + "| data: " + str(payload)) | ||
rel_log.debug(url + begin + "| data: " + str(payload)) | ||
r = requests.post(url + begin, data=payload, auth=auth) | ||
logging.debug(r.request.body) | ||
rel_log.debug(r.request.body) | ||
|
||
if r.status_code != 200: | ||
logging.error("HTTP code %d reported.", r.status_code) | ||
rel_log.error("HTTP code %d reported.", r.status_code) | ||
return False, "Internal" | ||
|
||
response = r.json() | ||
logging.debug(response) | ||
rel_log.debug(response) | ||
uuid = response['result']['data']['task_id'] | ||
logging.debug("Task accepted and given ID: %s", uuid) | ||
rel_log.debug("Task accepted and given ID: %s", uuid) | ||
result = False | ||
fail_type = None | ||
|
||
# It currently seems to take the onlide IDE API ~30s to process the compile request and | ||
# provide a response. Set the poll time to half that in case it does manage to compile | ||
# quicker. | ||
# It currently seems to take the onlide IDE API ~30s to process the compile | ||
# request and provide a response. Set the poll time to half that in case it | ||
# does manage to compile quicker. | ||
poll_delay = 15 | ||
logging.debug("Running with a poll for response delay of: %ss", poll_delay) | ||
rel_log.debug("Running with a poll for response delay of: %ss", poll_delay) | ||
|
||
# poll for output | ||
for check in range(polls): | ||
time.sleep(poll_delay) | ||
r = requests.get(url + "output/%s" % uuid, auth=auth) | ||
response = r.json() | ||
if response['result']['data']['task_complete']: | ||
|
||
|
||
data = response['result']['data'] | ||
if data['task_complete']: | ||
# Task completed. Now determine the result. Should be one of : | ||
# 1) Successful compilation | ||
# 2) Failed compilation with an error message | ||
# 3) Internal failure of the online compiler | ||
result = bool(response['result']['data']['compilation_success']) | ||
result = bool(data['compilation_success']) | ||
if result: | ||
logging.info("\t\tCompilation SUCCESSFUL\n") | ||
rel_log.info("COMPILATION SUCCESSFUL\n") | ||
else: | ||
# Did this fail due to a genuine compilation error or a failue of the api itself ? | ||
logging.info("\t\tCompilation FAILURE\n") | ||
fail_type = get_compilation_failure(response['result']['data']['new_messages']) | ||
# Did this fail due to a genuine compilation error or a failue of | ||
# the api itself ? | ||
rel_log.info("COMPILATION FAILURE\n") | ||
fail_type = get_compilation_failure(data['new_messages']) | ||
break | ||
else: | ||
logging.info("\t\tCompilation FAILURE\n") | ||
rel_log.info("COMPILATION FAILURE\n") | ||
|
||
if not result and fail_type == None: | ||
fail_type = "Internal" | ||
|
||
return result, fail_type | ||
|
||
|
||
def build_repo(target, program, user, pw, polls=25, url="https://developer.mbed.org/api/v2/tasks/compiler/"): | ||
""" Wrapper for sending an API command request to the online IDE. Sends a build request. | ||
def build_repo(target, program, user, pw, polls=25, | ||
url="https://developer.mbed.org/api/v2/tasks/compiler/"): | ||
""" Wrapper for sending an API command request to the online IDE. Sends a | ||
build request. | ||
|
||
Args: | ||
target - Target to be built | ||
|
@@ -192,11 +219,12 @@ def run_cmd(command, exit_on_failure=False): | |
Returns: | ||
result - True/False indicating the success/failure of the command | ||
""" | ||
logging.debug('[Exec] %s', ' '.join(command)) | ||
rel_log.debug('[Exec] %s', ' '.join(command)) | ||
return_code = subprocess.call(command, shell=True) | ||
|
||
if return_code: | ||
logging.warning("The command '%s' failed with return code: %s", (' '.join(command), return_code)) | ||
rel_log.warning("The command '%s' failed with return code: %s", | ||
(' '.join(command), return_code)) | ||
if exit_on_failure: | ||
sys.exit(1) | ||
|
||
|
@@ -217,22 +245,24 @@ def run_cmd_with_output(command, exit_on_failure=False): | |
result - True/False indicating the success/failure of the command | ||
output - The output of the command if it was successful, else empty string | ||
""" | ||
logging.debug('[Exec] %s', ' '.join(command)) | ||
rel_log.debug('[Exec] %s', ' '.join(command)) | ||
returncode = 0 | ||
output = "" | ||
try: | ||
output = subprocess.check_output(command, shell=True) | ||
except subprocess.CalledProcessError as e: | ||
logging.warning("The command '%s' failed with return code: %s", (' '.join(command), e.returncode)) | ||
rel_log.warning("The command '%s' failed with return code: %s", | ||
(' '.join(command), e.returncode)) | ||
returncode = e.returncode | ||
if exit_on_failure: | ||
sys.exit(1) | ||
return returncode, output | ||
|
||
def upgrade_test_repo(test, user, library, ref, repo_path): | ||
""" Upgrades a local version of a test repo to the latest version of its embedded library. | ||
If the test repo is not present in the user area specified in the json config file, then | ||
it will first be cloned. | ||
""" Upgrades a local version of a test repo to the latest version of its | ||
embedded library. | ||
If the test repo is not present in the user area specified in the json | ||
config file, then it will first be cloned. | ||
Args: | ||
test - Mercurial test repo name | ||
user - Mercurial user name | ||
|
@@ -243,15 +273,15 @@ def upgrade_test_repo(test, user, library, ref, repo_path): | |
Returns: | ||
updated - True if library was updated, False otherwise | ||
""" | ||
logging.info("Updating test repo: '%s' to SHA: %s", test, ref) | ||
rel_log.info("Updating test repo: '%s' to SHA: %s", test, ref) | ||
cwd = os.getcwd() | ||
|
||
repo = "https://" + user + '@developer.mbed.org/users/' + user + '/code/' + test | ||
|
||
# Clone the repo if it doesn't already exist | ||
path = abspath(repo_path + '/' + test) | ||
if not os.path.exists(path): | ||
logging.info("Test repo doesn't exist, cloning...") | ||
rel_log.info("Test repo doesn't exist, cloning...") | ||
os.chdir(abspath(repo_path)) | ||
clone_cmd = ['hg', 'clone', repo] | ||
run_cmd(clone_cmd, exit_on_failure=True) | ||
|
@@ -270,7 +300,7 @@ def upgrade_test_repo(test, user, library, ref, repo_path): | |
|
||
os.rename(lib_file, bak_file) | ||
else: | ||
logging.error("!! Error trying to backup lib file prior to updating.") | ||
rel_log.error("Failure to backup lib file prior to updating.") | ||
return False | ||
|
||
# mbed 2 style lib file contains one line with the following format | ||
|
@@ -279,7 +309,8 @@ def upgrade_test_repo(test, user, library, ref, repo_path): | |
lib_re = re.compile(exp) | ||
updated = False | ||
|
||
# Scan through mbed-os.lib line by line, looking for lib version and update it if found | ||
# Scan through mbed-os.lib line by line, looking for lib version and update | ||
# it if found | ||
with open(bak_file, 'r') as ip, open(lib_file, 'w') as op: | ||
for line in ip: | ||
|
||
|
@@ -297,16 +328,17 @@ def upgrade_test_repo(test, user, library, ref, repo_path): | |
# Setup the default commit message | ||
commit_message = '"Updating ' + library + ' to ' + ref + '"' | ||
|
||
# Setup and run the commit command. Need to use the rawcommand in the hglib for this in order to pass | ||
# the string value to the -m option. run_cmd using subprocess does not like this syntax. | ||
# Setup and run the commit command. Need to use the rawcommand in the hglib | ||
# for this in order to pass the string value to the -m option. run_cmd using | ||
# subprocess does not like this syntax. | ||
try: | ||
client.rawcommand(['commit','-m '+commit_message, lib_file]) | ||
|
||
cmd = ['hg', 'push', '-f', repo] | ||
run_cmd(cmd, exit_on_failure=True) | ||
|
||
except: | ||
logging.info("Lib file already up to date and thus nothing to commit") | ||
rel_log.info("Lib file already up to date and thus nothing to commit") | ||
|
||
os.chdir(cwd) | ||
return updated | ||
|
@@ -362,29 +394,44 @@ def get_latest_library_versions(repo_path): | |
|
||
return mbed, mbed_dev | ||
|
||
def log_results(lst, title): | ||
if len(lst) == 0: | ||
rel_log.info("%s - None", title) | ||
else: | ||
for entry in lst: | ||
rel_log.info("%s - Test: %s, Target: %s", title, entry[0], entry[1]) | ||
|
||
|
||
if __name__ == '__main__': | ||
|
||
parser = argparse.ArgumentParser(description=__doc__, | ||
formatter_class=argparse.RawDescriptionHelpFormatter) | ||
parser.add_argument('-l', '--log-level', help="Level for providing logging output", default='INFO') | ||
parser.add_argument('-l', '--log-level', | ||
help="Level for providing logging output", | ||
default='INFO') | ||
args = parser.parse_args() | ||
|
||
default = getattr(logging, 'INFO') | ||
level = getattr(logging, args.log_level.upper(), default) | ||
|
||
# Set logging level | ||
logging.basicConfig(level=level) | ||
rel_log = logging.getLogger("check-release") | ||
|
||
# Read configuration data | ||
json_data = json.load(open(os.path.join(os.path.dirname(__file__), "check_release.json"))) | ||
|
||
|
||
with open(os.path.join(os.path.dirname(__file__), "check_release.json")) as config: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lovely 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems to work fine using the rel_log instance globally , so not sure !! |
||
json_data = json.load(config) | ||
supported_targets = [] | ||
|
||
# Get a list of the officially supported mbed-os 2 targets | ||
for tgt in OFFICIAL_MBED_LIBRARY_BUILD: | ||
supported_targets.append(tgt[0]) | ||
|
||
|
||
if len(json_data["target_list"]) > 0: | ||
# Compile user supplied subset of targets | ||
supported_targets = json_data["target_list"] | ||
else: | ||
# Get a list of the officially supported mbed-os 2 targets | ||
for tgt in OFFICIAL_MBED_LIBRARY_BUILD: | ||
supported_targets.append(tgt[0]) | ||
|
||
config = json_data["config"] | ||
test_list = json_data["test_list"] | ||
repo_path = config["mbed_repo_path"] | ||
|
@@ -405,44 +452,57 @@ def get_latest_library_versions(repo_path): | |
mbed, mbed_dev = get_latest_library_versions(repo_path) | ||
|
||
if not mbed or not mbed_dev: | ||
logging.error("Could not obtain latest versions of library files!!") | ||
rel_log.error("Could not obtain latest versions of library files!!") | ||
exit(1) | ||
|
||
logging.info("Latest mbed lib version = %s", mbed) | ||
logging.info("Latest mbed-dev lib version = %s", mbed_dev) | ||
rel_log.info("Latest mbed lib version = %s", mbed) | ||
rel_log.info("Latest mbed-dev lib version = %s", mbed_dev) | ||
|
||
# First update test repos to latest versions of their embedded libraries | ||
for test in test_list: | ||
tests.append(test['name']) | ||
upgrade_test_repo(test['name'], user, test['lib'], mbed if test['lib'] == "mbed" else mbed_dev, repo_path) | ||
|
||
total = len(supported_targets)*len(tests) | ||
upgrade_test_repo(test['name'], user, test['lib'], | ||
mbed if test['lib'] == "mbed" else mbed_dev, | ||
repo_path) | ||
|
||
total = len(supported_targets) * len(tests) | ||
retries = 10 | ||
passes = 0 | ||
failures = [] | ||
skipped = [] | ||
|
||
# Compile each test for each supported target | ||
for test in tests: | ||
logging.info("Test compiling program: %s\n", test) | ||
rel_log.info("COMPILING PROGRAM: %s\n", test) | ||
for target in supported_targets: | ||
for retry in range(0, retries): | ||
logging.info("\tCompiling target: %s , attempt %u\n", target, retry) | ||
rel_log.info("COMPILING TARGET: %s , attempt %u\n", target, retry) | ||
result, mesg = build_repo(target, test, user, password) | ||
if not result: | ||
if mesg == 'Internal': | ||
# Internal compiler error thus retry | ||
continue | ||
else: | ||
# Genuine compilation error, thus print it out | ||
logging.error("\t\tError: %s\n", mesg) | ||
# Actual error thus move on to next compilation | ||
failures.append([test, target]) | ||
break | ||
|
||
passes += (int)(result) | ||
break | ||
else: | ||
logging.error("\t\tProgram/Target compilation failed due to internal errors. Removing from considered list!\n") | ||
rel_log.error("Compilation failed due to internal errors.\n") | ||
rel_log.error("Skipping test/target combination!\n") | ||
total -= 1 | ||
skipped.append([test, target]) | ||
|
||
rel_log.info(" SUMMARY OF COMPILATION RESULTS") | ||
rel_log.info(" ------------------------------") | ||
rel_log.info(" NUMBER OF TEST APPS: %d, NUMBER OF TARGETS: %d\n", | ||
len(tests), len(supported_targets)) | ||
log_results(failures, " FAILED") | ||
log_results(skipped, " SKIPPED") | ||
|
||
# Output a % pass rate, indicate a failure if not 100% successful | ||
pass_rate = int(passes/total) * 100 | ||
logging.info("Pass percentage = %d\n", pass_rate) | ||
pass_rate = (float(passes) / float(total)) * 100.0 | ||
rel_log.info(" PASS RATE %.1f %%\n", pass_rate) | ||
sys.exit(not (pass_rate == 100)) | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Nested json stuff can get gross.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a better name than
data
. but it matches the key...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we garenteed to have
response['result']['data']
here? it might be worth atry:
except KeyError:
or similar. Not blocking, but maybe we should consider it...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@theotherjimmy I believe it's a guaranteed response field