Add test for SorterStrategyDescriptor class

[yashpal2104]

Testing done

Add test for SorterStrategyDescriptor class

Aims to Fix JENKINS-69757

Submitter checklist

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira

[yashpal2104]

@MarkEWaite
I have implemented the TestSorterStrategy and TestSorterStrategyDescriptor classes to provide specific implementations for the abstract methods. Could you please review my approach to creating a concrete implementation of the abstract SorterStrategy class for testing purposes is it valid or not?
I want to ensure that this approach is valid and follows best practices for testing SorterStrategyDescriptor.

[MarkEWaite]

The technique looks good to me. I've submitted a few changes and corrections.

[MarkEWaite]

We use spotbugs annotations for nullability rather than JetBrains annotations

Suggested change

	import org.jetbrains.annotations.NotNull;
	import edu.umd.cs.findbugs.annotations.NonNull;

[yashpal2104]

Yes I think this was because I was using Intellij Jetbrains IDE, I mistakenly imported the incorrect one

[MarkEWaite]

Spotbugs annotation rather than JetBrains annotation

Suggested change

	public SorterStrategyCallback onNewItem(@NotNull Queue.Item item, SorterStrategyCallback weightCallback) {
	public SorterStrategyCallback onNewItem(@NonNull Queue.Item item, SorterStrategyCallback weightCallback) {

[yashpal2104]

Thanks for correcting this

[yashpal2104]

Thanks for reviewing the PR @MarkEWaite .
It is getting more and more difficult to write Tests for the classes without modifying the original src/main class because some methods and functions are missing for eg. The get function to retrieve or set functions, recently for MultiBucketStrategy there was a jenkins wrapper class(dont seem to remeber the name) which I needed to test so I was not able to add a dependency in the pom.xml (I think there was some sort of XStream Core dependency error I think the latest version was not able to be added). What are your thoughts on this? Do you think there needs modification in the original Source code and anything you would like to add?

[MarkEWaite]

It is getting more and more difficult to write Tests for the classes without modifying the original src/main class because some methods and functions are missing

Changes to the main classes are acceptable if they are:

• compatible, not breaking upgrades or other use cases
• minimally invasive, changing as little as they can while still improving testability

In the case of MultiBucketStrategy, why not focus on testing AbsoluteStrategy, since it extends MultiBucketStrategy?

I was not able to add a dependency in the pom.xml

I suspect you may have been making a mistake in that case. Generally you should not need to add to the plugin pom file unless the addition is test scoped and specific to that test.

[yashpal2104]

In the case of MultiBucketStrategy, why not focus on testing AbsoluteStrategy, since it extends MultiBucketStrategy?

Alright, I will try this instead

I suspect you may have been making a mistake in that case. Generally you should not need to add to the plugin pom file unless the addition is test scoped and specific to that test.

Ok I was only thinking of using that wrapper class for only specific test class only but a error of not having XStream-Core and a fix was provided by the IDE was to add this dependency link

[MarkEWaite]

Ok I was only thinking of using that wrapper class for only specific test class only but a error of not having XStream-Core and a fix was provided by the IDE was to add this dependency link

XStream is already provided by Jenkins core. There should never be a need to add it to a plugin pom.