Confused about test_get_project_issues_without_search_mixin and suggestions for Protocols

[cutekibry]

Firstly, there is a test called test_get_project_issues_without_search_mixin. I do not understand why it exists, since we always use Full JiraClient and not only ProjectMixins itself.

If there is some cases that search_issues is unavailable, then it should also be SearchMixins to handle that stuff and return None / raise RuntimeError. Then we test JiraClient.get_project_issues when search_issues is unavailable.

In my opinion, this test is unnecessary, and integration tests needs to blend the necessary mixins together, or just use JiraClient for convenience.

---

Secondly, there is a duplicate get_issue in EpicMixins. It should use get_issue from IssueMixins. The problem is obvious.

For linter errors, we can use Protocols (like Interface in other languages). Example: cutekibry@d627bf3

---

I understand refactoring is not so important but this problem is relatively easy to fix and if do not fix, it might cause tech debt and many problems.

For example:

• Change IssueMixins.get_issue might not effect JiraClient since there is a duplicate one in EpicMixins.
• SearchMixins.get_project_issues has a jql contains ORDER BY created DESC while ProjectMixins.get_project_issues does not. They even have different return types.
• Always need to call hasattr in each external function calling to pass the linter tests, even though they are not necessary.

If with agreement, I would remove unnecessary tests and duplicate functions in the future when I meet them.

[cutekibry]

Furthermore, I want to ask for one more thing: what is the standard to write tests for functions that involves multiple mixins?

For example, if ProjectMixins.get_project_issues involves SearchMixins.search_issues then we can:

• Treat it as Intergration test: Provide jira.jql.return_value to ProjectMixins | SearchMixins, or
• Treat it as Unit test: Provide search_issues.return_value manually to ProjectMixins, like the following code

mcp-atlassian/tests/unit/jira/test_projects.py

Lines 428 to 439 in 0f7736e

	def test_get_project_issues_with_search_mixin(projects_mixin):
	"""Test get_project_issues method with search_issues available."""
	# Mock the search_issues method
	mock_search_result = [MagicMock(), MagicMock()]
	projects_mixin.search_issues = MagicMock(return_value=mock_search_result)
	result = projects_mixin.get_project_issues("PROJ1", start=10, limit=20)
	assert result == mock_search_result
	projects_mixin.search_issues.assert_called_once_with(
	"project = PROJ1", start=10, limit=20
	)
	projects_mixin.jira.jql.assert_not_called()

---

Sorry for much interruption, but I sincerely wish the project better, although I might misunderstand something.

I am working at a company using Confluence/Jira, so in a long period I might have utter need for this project.

[sooperset]

@cutekibry, Thanks so much for the great feedback, and huge thanks for jumping in and doing some refactoring today!

You're absolutely right about the points you raised (duplication, refactoring needs). I prioritized speed initially, and now structural improvements are definitely needed.

Your contributions are incredibly helpful. Thanks again!