Skip to content

catch easyconfig parsing failure so we can generate and post a test report#4109

Merged
boegel merged 14 commits intoeasybuilders:developfrom
branfosj:20221023104456_new_pr_YHFGDApjoz
Nov 23, 2022
Merged

catch easyconfig parsing failure so we can generate and post a test report#4109
boegel merged 14 commits intoeasybuilders:developfrom
branfosj:20221023104456_new_pr_YHFGDApjoz

Conversation

@branfosj
Copy link
Copy Markdown
Member

@branfosj branfosj commented Oct 23, 2022

(created using eb --new-pr)

fixes #4102

If we fail during the parsing of the easyconfigs then we never get to the test report code, so we never upload a test report. During parsing of the easyconfigs we check for OS deps. This means that we do not get test reports from @boegelbot on easybuilders/easybuild-easyconfigs#16454

The report and messages using this PR generates:

@branfosj branfosj added this to the next release (4.6.3?) milestone Oct 23, 2022
@boegel boegel changed the title Catch ec parse fail so we can generate and post a test report catch easyconfig parsing failure so we can generate and post a test report Oct 25, 2022
Copy link
Copy Markdown
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

@branfosj We should make sure this enhancement is covered by the tests too, let me know if you need help with this.

Comment thread easybuild/main.py Outdated
Comment thread easybuild/tools/testing.py Outdated
Comment thread easybuild/tools/testing.py Outdated
@branfosj
Copy link
Copy Markdown
Member Author

branfosj commented Nov 9, 2022

@branfosj We should make sure this enhancement is covered by the tests too, let me know if you need help with this.

[remove old message]

Test added.

Comment thread easybuild/tools/testing.py Outdated
:param ecs_with_res: processed easyconfigs with build result (success/failure)
:param init_session_state: initial session state info to include in test report
:param pr_nrs: list of ints for the PRs in the easyconfigs repository
:param gist_log:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

gap in docstring?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Comment thread easybuild/tools/testing.py Outdated
:param success: boolean indicating whether all builds were successful
:param msg: message to be included in test report
:param init_session_state: initial session state info to include in test report
:param easyconfig_parse_error: error message for easyconfig parse failure
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

easyconfig_parse_error -> ec_parse_error

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Comment thread test/framework/toy_build.py Outdated
self.assertTrue(os.path.exists(test_report))
if fails:
test_result = 'FAIL'
if parse_error_regex:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

rename parse_error_regex to test_report_regex, and maybe make it a list (test_report_regexs) to allow passing in multiple patterns?

Copy link
Copy Markdown
Member Author

@branfosj branfosj Nov 23, 2022

Choose a reason for hiding this comment

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

Comment thread test/framework/toy_build.py
@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 23, 2022

@branfosj Merging of #4057 introduced a (somewhat difficult to resolve) merge conflict for easybuild/main.py, which I've resolved in branfosj#5

Copy link
Copy Markdown
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegel boegel merged commit 0add1ba into easybuilders:develop Nov 23, 2022
@branfosj branfosj deleted the 20221023104456_new_pr_YHFGDApjoz branch November 23, 2022 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test report not generated if there are missing OS dependencies

2 participants