Skip to content

filter out patched files in test/ in fetch_easyconfigs_from_pr#2547

Merged
akesandgren merged 2 commits intoeasybuilders:developfrom
boegel:fix_from_pr_test_files
Aug 13, 2018
Merged

filter out patched files in test/ in fetch_easyconfigs_from_pr#2547
akesandgren merged 2 commits intoeasybuilders:developfrom
boegel:fix_from_pr_test_files

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Aug 11, 2018

Without this fix, --from-pr fails on PRs that also change files in test/, for example:

eb --from-pr 6587 -D
== temporary log file in case of crash /tmp/eb-QVxdiy/easybuild-6UKT1p.log
ERROR: Couldn't find path to patched file /tmp/eb-QVxdiy/files_pr6587/test/easyconfigs/easyconfigs.py

@boegel boegel added the bug fix label Aug 11, 2018
@boegel boegel added this to the 3.6.3 milestone Aug 11, 2018
Comment thread test/framework/github.py Outdated
'animation-2.4-intel-2015b-R-3.2.1.eb',
]
try:
# PR for rename of ffmpeg to FFmpeg, see https://github.com/easybuilders/easybuild-easyconfigs/pull/2481/files
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (122 > 120 characters)

@boegel boegel force-pushed the fix_from_pr_test_files branch from a17dab8 to b1feab3 Compare August 11, 2018 11:56
@easybuilders easybuilders deleted a comment from boegelbot Aug 11, 2018
@easybuilders easybuilders deleted a comment from boegelbot Aug 11, 2018
@easybuilders easybuilders deleted a comment from boegelbot Aug 11, 2018
@akesandgren
Copy link
Copy Markdown
Contributor

There are now two occurrences of ec_files = gh.fetch_easyconfigs_from_pr( in test_fetch_easyconfigs_from_pr

Perhaps it should be rewritten to have a list (hash?) of tuples with PR and list-of-files outside "try" and then a for loop over the PRs. That would make it easier to add a new PR that need special testing in the future.
And it would probably be better to call sorted even on the manually generated list in case of clumsy patching.

@akesandgren
Copy link
Copy Markdown
Contributor

Not exactly what i meant by my comment but easy enough to change if there are more of them in the future.

Copy link
Copy Markdown
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

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

lgtm

@akesandgren akesandgren reopened this Aug 13, 2018
@easybuilders easybuilders deleted a comment from boegelbot Aug 13, 2018
@akesandgren akesandgren merged commit 2597fa0 into easybuilders:develop Aug 13, 2018
@boegel boegel deleted the fix_from_pr_test_files branch August 13, 2018 07:13
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.

3 participants