Skip to content

pass down stdin in 'import' check for extensions during sanity check#2276

Merged
ocaisa merged 5 commits intoeasybuilders:developfrom
boegel:run_cmd_inp_extensions_sanity_check
Aug 25, 2017
Merged

pass down stdin in 'import' check for extensions during sanity check#2276
ocaisa merged 5 commits intoeasybuilders:developfrom
boegel:run_cmd_inp_extensions_sanity_check

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Jul 13, 2017

By not passing down stdin into run_cmd in the sanity_check implementation for extensions, the 'import' check for R libraries is basically being skipped...

In addition, the caching mechanism for run_cmd should also take into account the inp argument.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Jul 13, 2017

This probably needs enhanced tests to verify the changes (and make sure the bug being fixed here isn't re-introduced).

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Jul 13, 2017

Without the changes to the Seurat easyconfig in easybuilders/easybuild-easyconfigs#4889, the sanity check fails as it should on top of this...

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Aug 7, 2017

tests are enhanced to catch the bug

@easybuilders/easybuild-framework-maintainers ready for review

ocaisa
ocaisa previously approved these changes Aug 25, 2017
Copy link
Copy Markdown
Member

@ocaisa ocaisa left a comment

Choose a reason for hiding this comment

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

Comment thread test/framework/run.py
self.assertEqual(True, run_cmd("echo hello", simple=True))
self.assertEqual(False, run_cmd("exit 1", simple=True, log_all=False, log_ok=False))

def test_run_cmd_cache(self):
Copy link
Copy Markdown
Member

@ocaisa ocaisa Aug 25, 2017

Choose a reason for hiding this comment

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

What about a test for the (caching of the) new inp argument to run_cmd?

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.

good point, added in #bc4be4154

@ocaisa ocaisa merged commit 16a392b into easybuilders:develop Aug 25, 2017
@boegel boegel deleted the run_cmd_inp_extensions_sanity_check branch August 25, 2017 14:59
boegel added a commit to boegel/easybuild-framework that referenced this pull request Aug 29, 2017
wpoely86 added a commit that referenced this pull request Aug 29, 2017
fix test_inject_checksums that got broken by changes to toy-0.0-gompi-1.3.12-test.eb test easyconfig in PR #2276
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants