Update test to verify using output, not in-task assertions, per testing guidelines by tresat · Pull Request #36874 · gradle/gradle

Update test to verify using output, not in-task assertions, per testing guidelines by tresat · Pull Request #36874 · gradle/gradle

tresat marked this pull request as ready for review

February 25, 2026 16:07

		attributes.keySet().each {
		attributesAsMap[it.name] = String.valueOf(attributes.getAttribute(it))
		}
		assert attributesAsMap.size() == 7

Choose a reason for hiding this comment

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

❌ I think this assertion should be preserved: it should be printed as verified in the test. As well as "standard" attributes

Applies to similar cases below

		abstract ConfigurableFileCollection getArtifacts()

		@Internal
		List<String> expectations = []

Choose a reason for hiding this comment

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

💅 this should be removed

		def 'consumer resolves jar variant of producer'() {
		file('consumer/build.gradle') << '''
		tasks.resolve {
		expectations = [ 'producer-jar.txt' ]

Choose a reason for hiding this comment

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

💅 These expectations are no longer needed, right?

		def files = configurations.runtimeClasspath
		doLast {
		assert files*.name == ["foo-1.0.jar", "${expectedFile}"]
		println files*.name

Choose a reason for hiding this comment

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

💅 Would be nice to prefix with something like "Resolved:"

		outputContains("artifact3.name=dist")
		outputContains("artifact3.type=zip")
		outputContains("artifact3.extension=zip")
		outputContains("artifact3.classifier=null")

Choose a reason for hiding this comment

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

💅 I think here the check should be a bit smarter, since 'null' will also pass it