add end-to-end test for --minimal-toolchains #1619

boegel · 2016-02-17T18:14:55Z

This reproduces the issue in #1606.

Without #1614 included in develop, this test will fail.

hpcugentbot · 2016-02-17T18:21:23Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2711/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

boegel · 2016-02-17T18:59:01Z

Test fails as it should:

ERROR [0.365s]: End-to-end test for --minimal-toolchains.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test/framework/options.py", line 2104, in test_minimal_toolchains
    self.eb_main(args, do_build=True, raise_error=True, testing=False)
  File "test/framework/utilities.py", line 242, in eb_main
    raise myerr
EasyBuildError: "Failed to process easyconfig /var/tmp/eb-_sxVRh/eb-JRorAV/eb-YKAHI_/eb-rmtr9f/eb-6H7qEN/eb-iSOisD/eb-8K9O5o/eb-T4sxWA/test_minimal_toolchains.eb: Failed to find easyconfig file 'hwloc-1.6.2-gompi-1.4.10.eb' when determining module name for: {'external_module_metadata': {}, 'full_mod_name': None, 'build_only': False, 'versionsuffix': '', 'external_module': False, 'dummy': False, 'toolchain': {'version': '1.4.10', 'name': 'gompi'}, 'name': 'hwloc', 'short_mod_name': None, 'version': '1.6.2', 'hidden': False}"

----------------------------------------------------------------------
Ran 387 tests in 366.827s

FAILED (errors=1)

boegel · 2016-02-17T19:02:19Z

let's retest now that #1614 is merged...

Jenkins: test this please

boegel · 2016-02-17T19:10:55Z

easybuild/framework/easyconfig/easyconfig.py

@@ -162,7 +162,7 @@ def get_toolchain_hierarchy(parent_toolchain):
        for dep in parsed_ec['dependencies']:
            ec = robot_find_easyconfig(dep['name'], det_full_ec_version(dep))
            ec = process_easyconfig(ec, validate=False)[0]
-            alldeps = [ec['ec']['toolchain']] + ec['ec']['dependencies']
+            alldeps = [ec['ec']['toolchain']] + parsed_ec['dependencies']


@ocaisa: the fix in #1576 was not 100% correct, as I discovered when adding a test for it...

cc @damianam

You sure this is correct? I think this was originally almost correct - what was the error? I think it should probably be

alldeps = [ec['ec']['toolchain']] + ec['ec']['dependencies'] + [dep['toolchain'] for dep in ec['ec']['dependencies']]

Argh, I'm confused by this! But I think the line above will solve it. Your change is not focussed enough on toolchains. We only care about what toolchains are being used, the proposed line looks at the toolchains of deps, the deps of deps and the toolchains of the deps of deps. We will overpopulate the list but that's ok, because we do a set later.

@ocaisa: the problem is that the previous line was taking the dependencies of the dependencies...

Say we're parsing GCC-4.9.3-2.25.eb; the dependencies are GCCcore and binutils, which are listed in parsed_ec['dependencies']

We also want to take into account the toolchain for each of these dependencies, so [ec['ec']['toolchain']]

We're adding the same list of deps over and over again now, so I need to change this to:

deps_tcs = [d['name'] for d in parsed_ec['dependencies']] for dep in parsed_ec['dependencies']: # also add toolchains of each dep

or something like that (I'll look into it later)

Ok, but you're going backwards. The reason this was changed originally in #1576 was for iccifort built on GCCcore. iccifort depends on icc/ifort which then depend on GCCcore/binutils. Your proposal wouldn't cover this case. You need toolchains of deps and deps of deps and toolchains of deps of deps. Yes this is overkill but who cares, you've already parsed the deps anyway and we select only the toolchain we're interested in later and then make sure we have uniqueness.

OK, I'll add a testcase for iccifort and make sure that works too.

hpcugentbot · 2016-02-17T19:26:34Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2714/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

hpcugentbot · 2016-02-17T19:37:08Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2715/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

hpcugentbot · 2016-02-17T21:36:41Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2716/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

ocaisa · 2016-02-18T09:01:41Z

test/framework/robot.py

@@ -567,6 +567,14 @@ def test_get_toolchain_hierarchy(self):
            {'name': 'gompi', 'version': '1.4.10'},


gompi is a bit of a broken choice when you try to add dummy, because the underlying GCC doesn't actually have any dependencies on anything built with dummy which is (currently) required. I guess we should hard-include dummy when the relevant option is there

This would solve the first failing test

Wait, this problem is also the cause of the second test failure, it must be related to the change you made in the toolchain hierarchy generator

It's the caching for toolchain hierarchy screwing up the tests... We'll need to reset the cache.

I don't think gompi is a broken choice, because we always include the toolchain too, not only the deps.

So, dummy is considered as a subtoolchain because GCC is built with dummy

Agreed, my mistake

On 18 Feb 2016 21:06, "Kenneth Hoste" <notifications@github.com mailto:notifications@github.com> wrote:

In test/framework/robot.pyhttps://github.com//pull/1619#discussion_r53374477:

@@ -567,6 +567,14 @@ def test_get_toolchain_hierarchy(self):
{'name': 'gompi', 'version': '1.4.10'},

I don't think gompi is a broken choice, because we always include the toolchain too, not only the deps.

So, dummy is considered as a subtoolchain because GCC is built with dummy

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1619/files#r53374477.

Forschungszentrum Juelich GmbH
52425 Juelich
Sitz der Gesellschaft: Juelich
Eingetragen im Handelsregister des Amtsgerichts Dueren Nr. HR B 3498
Vorsitzender des Aufsichtsrats: MinDir Dr. Karl Eugen Huthmacher
Geschaeftsfuehrung: Prof. Dr.-Ing. Wolfgang Marquardt (Vorsitzender),
Karsten Beneke (stellv. Vorsitzender), Prof. Dr.-Ing. Harald Bolt,

Prof. Dr. Sebastian M. Schmidt

Test minimal toolchains

…_hierarchy Allow cache clearing for toolchain hierarchy

hpcugentbot · 2016-02-18T14:31:51Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2722/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

Add the cache clearing

…_hierarchy Update robot.py

hpcugentbot · 2016-02-18T17:38:05Z

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2725/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

hpcugentbot · 2016-02-18T20:40:54Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2728/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

hpcugentbot · 2016-02-18T20:47:15Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2729/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

hpcugentbot · 2016-02-18T22:52:54Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2731/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

hpcugentbot · 2016-02-18T23:56:05Z

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2732/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

boegel · 2016-02-19T07:34:21Z

@ocaisa: can you give this another look? Should be good to go.

ocaisa · 2016-02-19T08:25:36Z

easybuild/framework/easyconfig/easyconfig.py

                                 current_tc_name, current_tc_version)

        # parse the easyconfig
        parsed_ec = process_easyconfig(path, validate=False)[0]

        # search the toolchain+dependencies for the version of the subtoolchain
-        dep_tcs = []
+        dep_tcs = [d for d in parsed_ec['dependencies'] if d['name'] == subtoolchain_name]


I know this is a little pedantic since we have solved the particular problem with icc/ifort but...
Originally this function only dealt with toolchains from the depenencies, now it is dealing with dependencies and the toolchains of dependencies but these are not the same. Dependencies can have a versionsuffix which toolchains don't, so if we are adding dependencies we need to add them as

{'name': d['name'], 'version': '%s%s' % (d['version'], d['versionsuffix'])}

Same is true when adding dependencies below (basically need a repeat of this line). And I still think we should also be adding toolchains of deps of deps, since we've already parsed the deps it comes at zero cost (for example, if you had decided to comment out the GCCcore dependency rather than binutils in icc/ifort we wouldn't have picked up the GCCcore version even though it is available as the toolchain of binutils).

OK, makes sense to me, thanks for the feedback.

@ocaisa: fixed now, please rererererererererereview?

ocaisa · 2016-02-19T15:01:07Z

lgtm
If it passes the tests, I'm happy

hpcugentbot · 2016-02-19T15:39:45Z

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2735/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

hpcugentbot · 2016-02-19T16:40:29Z

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2736/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

boegel · 2016-02-19T17:16:04Z

Going in, thanks for the excellent feedback @ocaisa!

add end-to-end test for --minimal-toolchains

add end-to-end test for --minimal-toolchains

1507de3

boegel added this to the v2.7.0 milestone Feb 17, 2016

boegel mentioned this pull request Feb 17, 2016

--minimal-toolchains seems to don't work with ToolchainMNS #1606

Closed

boegel mentioned this pull request Feb 17, 2016

EasyBuild 2.7.0 (roadmap) easybuilders/easybuild#192

Closed

17 tasks

boegel added 4 commits February 17, 2016 20:07

Merge branch 'develop' into test_minimal_toolchains

8ee1df8

fix error message

c6bcfbe

add test case for GCCcore being considered as subtoolchain for GCC

2563e7c

fix also considering dependencies when looking for subtoolchains

42df574

boegel reviewed Feb 17, 2016
View reviewed changes

fix broken tests

b144735

ocaisa reviewed Feb 18, 2016
View reviewed changes

Alan and others added 3 commits February 18, 2016 14:26

Add ability to clear toolchain cache

3b8bcc0

Merge pull request #32 from boegel/test_minimal_toolchains

5d4a32f

Test minimal toolchains

Merge pull request #13 from ocaisa/allow_cache_clearing_for_toolchain…

c4ed44f

…_hierarchy Allow cache clearing for toolchain hierarchy

boegel mentioned this pull request Feb 18, 2016

Add ability to clear toolchain cache #1621

Closed

ocaisa and others added 2 commits February 18, 2016 15:33

Update robot.py

78dd236

Add the cache clearing

Merge pull request #14 from ocaisa/allow_cache_clearing_for_toolchain…

a9fb4cb

…_hierarchy Update robot.py

boegel mentioned this pull request Feb 18, 2016

Fix broken file name #1622

Merged

boegel added 3 commits February 18, 2016 21:00

remove faulty comments w.r.t. toolchain hierarchy cache

9dc6e92

also include SQLite dep in test_minimal_toolchains

2fa7f79

fix get_toolchain_hierarchy & add extra test

1cfb155

add more test easyconfigs used in get_toolchain_hierarchy test

5ffb25b

comment out irrelevant binutils deps in icc/ifort test easyconfigs

de54ed6

fix broken test

c5be45a

ocaisa reviewed Feb 19, 2016
View reviewed changes

boegel added 2 commits February 19, 2016 15:56

rewrite get_toolchain_hierarchy to fix @ocaisa's remarks

d2f9ea8

very minor style fix

42f3bfa

boegel added a commit that referenced this pull request Feb 19, 2016

Merge pull request #1619 from boegel/test_minimal_toolchains

259a59f

add end-to-end test for --minimal-toolchains

boegel merged commit 259a59f into easybuilders:develop Feb 19, 2016

boegel deleted the test_minimal_toolchains branch February 19, 2016 17:16

boegel mentioned this pull request Jan 31, 2017

fix broken error message in get_toolchain_hierarchy + dedicated test case #2099

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add end-to-end test for --minimal-toolchains #1619

add end-to-end test for --minimal-toolchains #1619

boegel commented Feb 17, 2016

hpcugentbot commented Feb 17, 2016

boegel commented Feb 17, 2016

boegel commented Feb 17, 2016

boegel Feb 17, 2016

ocaisa Feb 18, 2016

ocaisa Feb 18, 2016

boegel Feb 18, 2016

ocaisa Feb 18, 2016

boegel Feb 18, 2016

hpcugentbot commented Feb 17, 2016

hpcugentbot commented Feb 17, 2016

hpcugentbot commented Feb 17, 2016

ocaisa Feb 18, 2016

ocaisa Feb 18, 2016

ocaisa Feb 18, 2016

boegel Feb 18, 2016

boegel Feb 18, 2016

ocaisa Feb 18, 2016

hpcugentbot commented Feb 18, 2016

hpcugentbot commented Feb 18, 2016

hpcugentbot commented Feb 18, 2016

hpcugentbot commented Feb 18, 2016

hpcugentbot commented Feb 18, 2016

hpcugentbot commented Feb 18, 2016

boegel commented Feb 19, 2016

ocaisa Feb 19, 2016

boegel Feb 19, 2016

boegel Feb 19, 2016

ocaisa commented Feb 19, 2016

hpcugentbot commented Feb 19, 2016

hpcugentbot commented Feb 19, 2016

boegel commented Feb 19, 2016

		@@ -567,6 +567,14 @@ def test_get_toolchain_hierarchy(self):
		{'name': 'gompi', 'version': '1.4.10'},

add end-to-end test for --minimal-toolchains #1619

add end-to-end test for --minimal-toolchains #1619

Conversation

boegel commented Feb 17, 2016

hpcugentbot commented Feb 17, 2016

boegel commented Feb 17, 2016

boegel commented Feb 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hpcugentbot commented Feb 17, 2016

hpcugentbot commented Feb 17, 2016

hpcugentbot commented Feb 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Prof. Dr. Sebastian M. Schmidt

hpcugentbot commented Feb 18, 2016

hpcugentbot commented Feb 18, 2016

hpcugentbot commented Feb 18, 2016

hpcugentbot commented Feb 18, 2016

hpcugentbot commented Feb 18, 2016

hpcugentbot commented Feb 18, 2016

boegel commented Feb 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocaisa commented Feb 19, 2016

hpcugentbot commented Feb 19, 2016

hpcugentbot commented Feb 19, 2016

boegel commented Feb 19, 2016