Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rft build config #673

Merged
merged 1 commit into from
Nov 1, 2021
Merged

Rft build config #673

merged 1 commit into from
Nov 1, 2021

Conversation

baev
Copy link
Member

@baev baev commented Oct 28, 2021

Context

Checklist

@baev
Copy link
Member Author

baev commented Oct 28, 2021

@vlsi could you please review, sir?

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

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
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

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
Member Author

Choose a reason for hiding this comment

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

Copy link

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
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

vlsi commented Oct 28, 2021

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

@baev
Copy link
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

@@ -215,6 +215,17 @@ configure(libs) {
}
}

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

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
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