Skip to content

Only download test data if running the test that needs them#402

Merged
peremato merged 2 commits into
root-project:masterfrom
ellert:testdata
Mar 18, 2017
Merged

Only download test data if running the test that needs them#402
peremato merged 2 commits into
root-project:masterfrom
ellert:testdata

Conversation

@ellert
Copy link
Copy Markdown
Contributor

@ellert ellert commented Mar 8, 2017

The TestData target is currently declared ALL, which means it is always executed during the build.
However, the data it downloads is only used for running the stressProof test, so if this test is not run the downloaded data files are not needed.

By removing the TestData target from ALL and making it a requirement of the test-stressproof target instead, the files are only downloaded if they are needed. Disabling the stressProof test now also disables the download of the data files. This is important when building in an environment without network access.

@phsft-bot
Copy link
Copy Markdown

Can one of the admins verify this patch?

@ellert ellert mentioned this pull request Mar 8, 2017
@peremato peremato self-assigned this Mar 8, 2017
@peremato
Copy link
Copy Markdown
Contributor

Hi Mattias. I do not think this works. The DEPENDS in ROOT_ADD_TEST() is intended for adding dependencies between tests. The solution is probably adding a 'pre-command' to the test test-stressproof that builds the target TestData before running the actual test.

@peremato
Copy link
Copy Markdown
Contributor

@phsft-bot build

@peremato peremato merged commit ff782c9 into root-project:master Mar 18, 2017
@ellert ellert deleted the testdata branch March 20, 2017 12:27
gganis pushed a commit to gganis/root that referenced this pull request Apr 3, 2017
…ject#402)

* Only download test data if running the test that needs them

* Copy test data in the pre-command of the test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants