Skip to content

Rft build config#673

Merged
baev merged 1 commit into
masterfrom
rft-allure-cfg
Nov 1, 2021
Merged

Rft build config#673
baev merged 1 commit into
masterfrom
rft-allure-cfg

Conversation

@baev
Copy link
Copy Markdown
Member

@baev baev commented Oct 28, 2021

Context

Checklist

@baev
Copy link
Copy Markdown
Member Author

baev commented Oct 28, 2021

@vlsi could you please review, sir?

Comment thread build.gradle.kts
clean = true
resultsDirs = subprojects.map { file("${it.buildDir}/allure-results") }.filter { it.exists() }
repositories {
mavenLocal()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is it worth adding mavenLocal by default? It makes the build non-reproducible

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.

we should be fine. Haven't seen any issues

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why do you add it?
It is a time bomb: developers might have unknown jars in mavenLocal repo, so consuming from mavenLocal leads to non-reproducible results.

If you really need mavenLocal for some reason, consider adding a property so mavenLocal is added only in case user explicitly opts-in.

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why adding mavenLocal though? By the way, Gradle always checks jars (if they have proper contents) in .m2 even in case mavenLocal() is not mentioned, so mavenLocal() does NOT make the build faster. However, it makes the build less reliable, so it is better to avoid mavenLocal by default.

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.

nah, I just always committing it accidentally with an other changes. It's only needed to develop a new feature/integration against some snapshot dependency (or even when I plan to patch some framework or so)

And, as is was there forever (and not only there -- as far as I remember maven uses it always by default) I feel a bit sceptical about removing it just because some bad thing could probably happen in future

@vlsi
Copy link
Copy Markdown

vlsi commented Oct 28, 2021

Looks good. Do you mean the agent is added by allure-gradle plugin?

@baev
Copy link
Copy Markdown
Member Author

baev commented Oct 29, 2021

Do you mean the agent is added by allure-gradle plugin?

yep.

Encountered few issues with it though

  1. Had to specify autoconfigureListeners.set(true) because otherwise dependencySubstitution for spi-off is triggered. Probably it worth hardcoding exact dependency coordinates that support that. This is only happens for dependencies within io.qameta.allure group, .
  2. allureAggregateServe doesn't work for some reason. It just stuck without any message

Comment thread build.gradle.kts

allure {
adapter {
autoconfigure.set(false)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

why do you set "autoconfigure=false"?

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.

because we use junit platform to test other test frameworks in most cases.

@baev baev merged commit 2e38ce2 into master Nov 1, 2021
@baev baev deleted the rft-allure-cfg branch November 1, 2021 16:09
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.

3 participants