fix(#126): Improves build time

[lordofthejars]

Short description of what this resolves:

Improves build time by ignoring overlapping tests and narrow builds

Changes proposed in this pull request:

• Ignores tests that are overlapped with other tests. Currently they are just ignored.
• Narrow build to modules that received change.

Fixes #126

[bartoszmajsak]

Can you link to the redundant test? Hard to find on the mobile ;) I also don't feel confident about having ignored test as a solution. Maybe we could evaluate the overlap and decide what to change as part of this PR?

[lordofthejars]

Ignored tests it is a partial solution before we agreed that they are really redundant. Just that I have not missed something. El 2 oct. 2017 9:54 p. m., "Bartosz Majsak" <[email protected]> escribió:

…

Can you link to the redundant test? Hard to find on the mobile ;) I also don't feel confident about having ignored test as a solution. Maybe we could evaluate the overlap and decide what to change as part of this PR? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#203 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABcmYX0jin5DLJizw9evUjQaRrNsGhIIks5soT9ygaJpZM4Pq6RM> .

[lordofthejars]

• https://github.com/arquillian/smart-testing/blob/master/functional-tests/test-bed/src/test/java/org/arquillian/smart/testing/ftest/affected/HistoricalChangesAffectedTestsSelectionExecutionFunctionalTest.java should be removed. In this case we are testing that a test is detected between two commits but then there is another test https://github.com/arquillian/smart-testing/blob/master/functional-tests/test-bed/src/test/java/org/arquillian/smart/testing/ftest/affected/ChangesOnDifferentModulesAffectedTestsSelectionExecutionFunctionalTest.java that checks exactly the same but when changes occurs also in other modules, so effectively the functional test of checking the tests between a range of commits are tested in both and the latter one fully overlaps with the first one, so for this reason there is no rewrite but a remove of the test. (Now ignored and when we agree removed)

• https://github.com/arquillian/smart-testing/blob/master/functional-tests/test-bed/src/test/java/org/arquillian/smart/testing/ftest/failed/HistoricalChangesFailedTestsSelectionExecutionFunctionalTest.java test should be ignored because failed strategy does not use any local or scm changes, just gets previous failures, so this test can be removed.

[MatousJobanek]

I'm not sure if ignoring the tests is the best solution. Maybe putting these semi-redundant test classes into another test suite that will be executed nightly. But yeah, if we agree on that they are redundant, we can remove them.
As for the HistoricalChangesFailedTestsSelectionExecutionFunctionalTest.java test - is that really a redundant test, or just a bug?

I believe that we haven't used all possibilities of parallelism. I've been playing with it some time ago - I'll share a link with my work/attempt.

[lordofthejars]

HistoricalChangesFailedTestsSelectionExecutionFunctionalTest.java

It is a bug or a none sense test. Failed strategy does not check historical scm data, so this test can be removed because it is testing exactly the same as the other one.

[lordofthejars]

ignoring the tests is the best solution

As I said is a temporal solution until we decide to remove them

semi-redundant test

In this concrete class of HistoricalChangesAffectedTestsSelectionExecutionFunctionalTest.java I'd delete it because the logic executed in HistoricalChangesAffectedTests.... is exactly the same as ChangesOnDifferentModules... test.

[MatousJobanek]

Check this commit: MatousJobanek@02d9197

I'm combining several things there (the numbers of threads are just example):

• the test-bed build uses Maven parallelism with 50 threads per cpu core - for more information take a look here

• the same is used for build invocation on Travis (could be used locally)

• the Travis is building in three virtual machines at once - check this:

  • one for unit tests
  • one for functional tests - to separate unit tests and functional tests I had to introduce two profiles inside of the pom files
  • one for generating documentation - no to do it in the previous builds/vm twice

• speeding up Java startup using properties: -XX:+TieredCompilation -XX:TieredStopAtLevel=1

The local build mvn clean install -T 100 now takes 04:28
The build on travis:

Ran for 14 min 41 sec
Total time 18 min 41 sec 

see here: https://travis-ci.org/MatousJobanek/smart-testing/builds/282751554

[MatousJobanek]

I'll send it as a PR if you're fine with it...

[lordofthejars]

Of course I am fine, it is our PR initially implemented by me :) El 3 oct. 2017 4:38 p. m., "Matous Jobanek" <[email protected]> escribió:

…

I'll send it as a PR if you're fine with it... — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#203 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABcmYeqzD3Bsp6PJ9lcDgtZb64i-emM0ks5sokbWgaJpZM4Pq6RM> .

[MatousJobanek]

Here it is - as a separated PR: #204
This one is still yours :-)
I went through the tests you ignored and I'm fine with removing the failed one: HistoricalChangesFailedTestsSelectionExecutionFunctionalTest.java
But I would keep the HistoricalChangesAffectedTestsSelectionExecutionFunctionalTest.java as it tests on different granularity than the other one.

[lordofthejars]

Ok, it is fine for me El 3 oct. 2017 5:04 p. m., "Matous Jobanek" <[email protected]> escribió:

…

Here it is - as a separated PR: #204 <#204> This one is still yours :-) I went through the tests you ignored and I'm fine with removing the failed one: HistoricalChangesFailedTestsSelectionExecutionFunctionalTest.java But I would keep the HistoricalChangesAffectedTests SelectionExecutionFunctionalTest.java as it tests on different granularity than the other one. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#203 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABcmYYYgHSfmc5zcd1QZQsRhOBXxAMGdks5sok0YgaJpZM4Pq6RM> .

[lordofthejars]

@MatousJobanek updated. If you agree we can merge.

[bartoszmajsak]

I just wanted to catch up on it - I don't see an approval from Matous and yet you asked him

If you agree we can merge.

Did you guys took it offline or it was just merged?

[lordofthejars]

@bartoszmajsak we talked on mattermost.

[bartoszmajsak]

How much did we improve the build time after this PR got merged @lordofthejars ?