Build task should not depend on instanceCheck

[pun-ky]

gradle-aem-multi / sh gradlew :build

[pun-ky]

it is handy to use both checks instanceCheck and environmentCheck by running simply check task but... Gradle users used to have artifact build and e.g unit tests only run by running simply build, we have a collision here, building artifacts only should not run instance / environment checking (remote relationships)

[pun-ky]

current workaround (or acceptable solution?) for this is to simply add -Poffline=true as it is done on CI

[pun-ky]

in my opinion, nothing to be changed, we need to just assume that check lifecycle tasks checks all that could be checked / runs unit tests, integration tests and any other tests

[pun-ky]

closing as documented https://github.com/Cognifide/gradle-aem-plugin#building-artifacts-on-ci-server--offline-mode

[paul-bjorkstrand]

Late to the party, but I want to put in my opinion. I would argue that since build does not do anything with the server, having check depend on instanceCheck is, at best, misguided. I was a bit confused when my builds were not successful with my instances down. While this can be worked around with both -Poffline=true or -x instanceCheck (Gradle's way to skip a task), I think that it makes little sense to be a part of build.

I feel this is more natural to developers than to have an obscure (though documented) flag to set it as in "offline mode".

[pun-ky]

Thx for your opinion.

The thing is that I am balancing here with comfortable usage for Gradle newbies / check task just checks everything that could be checked. But yes then in such scenario having instances up is kind of prerequisite. Generally using pługin without having instances up is quite rare case so that this extra parameter to be specified is only required in such specific case.

I could remove that dependency so that build / check task will no longer depend on instanceCheck and environmentCheck but still it would be nice to have some single lifecycle task running both instanceCheck and environmentCheck tasks but in Gradle lifecycle plugin there is no such task to attach to. I could introduce e.g verify task but then renaming task to environmentVerify and instanceVerify will be required to have it all consistent.

What do you think? @paul-bjorkstrand

[paul-bjorkstrand]

I'm good with that approach, and I understand the balance. My comment was purely out of confusion: "why is the build taking so long?" and when I let it continue to completion "why did the build fail?" I was able to figure out the problem, thanks to my experience with Gradle (and thanks to a nifty task-tree plugin).

I agree that most people will work online, with an AEM instance running, but sometimes you just want to make a quick Java change, run your build/tests and be done, without the need to spin up an instance (I get these spurts of inspiration late at night, and I don't want to restart my instances constantly :grin:). I'm good with just using the flag you provided, or using my build -x instanceCheck, but for the benefit of others, I think it should be more prominently featured in the docs.

I only found this flag after coming here to open an issue, almost identical to this.

[pun-ky]

but sometimes you just want to make a quick Java change, run your build/tests and be done

and in that case Gradle build users (just like you) used to use just build task, not assemble or jar etc ;) so that you are also right here.

now I am confused which names are good for these tasks... check was just semantically good ;)

so maybe historically, previously we had (GAP 6.x) aemAwait -> instanceCheck -> ... so maybe instanceAwait once again will be good? will not indicate corelation with check

instanceCheck -> instanceAwait environmentCheck -> environmentAwait await.dependsOn(instanceAwait, environmentAwait) // lifecycle task await

[paul-bjorkstrand]

I think that makes sense (logically and linguistically. I think the biggest problem with having build servers & people like me move to assemble or jar, is they don't run tests. I prefer to run (unit) tests every time I build (yeah, I am that kind of nutcase 😆). I appreciate the dialogue.

[pun-ky]

so you may now expect that changes mentioned in https://github.com/Cognifide/gradle-aem-plugin/issues/426#issuecomment-524571539 will be done in 7.1.0 (within next few weeks hope so)

[pun-ky]

implementation / PR https://github.com/Cognifide/gradle-aem-plugin/pull/445 @paul-bjorkstrand

[pun-ky]

implemented, to be released in 7.1.0