Skip to content
This repository was archived by the owner on Sep 15, 2025. It is now read-only.

Conversation

@crazytonyli
Copy link
Contributor

Description

What says in the title. No library code was changed in this PR.

The added unit tests stub many different kinds of HTTP requests sent by the XMLRPC endpoint discovery code (WordPressOrgXMLRPCValidator). I'm not happy about their current state though: they are too verbose and the test functions are not very easy to read. I'm hoping to make some helper functions to so that the test functions can be concise enough to read, but I'll leave that to next year. 😄

Testing Details

None needed. This PR only adds unit tests.


  • Please check here if your pull request includes additional test coverage.
  • I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

Copy link
Contributor

@mokagio mokagio left a comment

Choose a reason for hiding this comment

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

Appreciate the thorough testing. Thank you.

The tests are indeed a bit verbose. I couple of syntax sugar suggestions from me:

  • Push https://www.apple.com into a let property to DRY
  • Add some helpers for repeated stubs like stub(condition: isHost("www.apple.com") && isPath("..."))...
  • Some of the async expectations could simplified with Nimble's waitUntil, but those that have enforceOrder might need to remain vanilla XCTest

None of these suggestions is particularly life-changing, IMHO. So, for what is worth, I think we can live with these verbose but comprehensive tests.

@mokagio
Copy link
Contributor

mokagio commented Jan 11, 2024

None of these suggestions is particularly life-changing, IMHO. So, for what is worth, I think we can live with these verbose but comprehensive tests.

On top of that. Is is my understanding that we'd like to drop XMLRPC sooner rather than later, so I wouldn't advocate spend too much time on it.

@crazytonyli crazytonyli merged commit 6fdba28 into trunk Jan 11, 2024
@crazytonyli crazytonyli deleted the refactor-xml-rpc-error-1 branch January 11, 2024 02:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants