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

Update Task Thin Executions list function for parity with 2.11.x #6063

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

corneil
Copy link
Contributor

@corneil corneil commented Nov 12, 2024

Updated task thin executions link handing and added extra test for parity with 2.11.x

See #6062

Updated task thin executions link handing and added extra test for parity with 2.11.x

See spring-cloud#6062
@corneil corneil added the review/at-least-one Only one reviewer is required to merge this PR. label Nov 12, 2024
@corneil corneil requested review from cppwfs and onobc November 12, 2024 08:25
Updated task thin executions link handing and added extra test for parity with 2.11.x.
Update minimum supported version of client.

See spring-cloud#6062
Updated task thin executions link handing and added extra test for parity with 2.11.x.
Update minimum supported version of client.

See spring-cloud#6062
Updated task thin executions link handing and added extra test for parity with 2.11.x.
Update minimum supported version of client.

See spring-cloud#6062
@corneil corneil requested review from onobc and cppwfs and removed request for cppwfs and onobc November 12, 2024 13:41
Copy link
Contributor

@cppwfs cppwfs left a comment

Choose a reason for hiding this comment

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

Looks good just a couple of nitpicks

Since you are bringing this class up to date can you remove the following since they are not being used:

  • the AboutResource import
  • actualDataFlowServerCoreVersion
  • aboutLink

Thanks!

validateExecutionLinkPresent("1.8.0");
validateExecutionLinkPresent("1.9.0");
validateExecutionLinkPresent("2.0.0");
validateExecutionLinkPresent("3.0.0");
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably want to have negative test as well. i.e. test with an old version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment we don't have old versions we support with links missing

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking of something like:

	@Test
	void invalidDataFlow() {
		assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> validateExecutionLinkPresent("2.11.5"));
	}
	```

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

Updated task thin executions link handing and added extra test for parity with 2.11.x.
Update minimum supported version of client.
Added test for invalid old version.

See spring-cloud#6062
@corneil corneil requested a review from cppwfs November 14, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review/at-least-one Only one reviewer is required to merge this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants