GeezerVison

@asottile

Do you have any ideas how to explain the test failure?

   FAIL: test_default (test.test_xmlrpc.DateTimeTestCase.test_default)
   ----------------------------------------------------------------------
   Traceback (most recent call last):
     File "/Users/runner/Library/Developer/CoreSimulator/Devices/CED4C8DA-56B5-4FBF-A288-C5C2C197B3AD/data/Containers/Bundle/Application/4B219D99-CC21-4E94-B496-5F175FEC5822/iOSTestbed.app/python/lib/python3.15/test/test_xmlrpc.py", line 487, in test_default
       self.assertEqual(str(t),
       ~~~~~~~~~~~~~~~~^^^^^^^^
                        time.strftime("%Y%m%dT%H:%M:%S", localtime))
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   AssertionError: '20260331T20:39:38' != '20130715T00:24:49'
   - 20260331T20:39:38
   + 20130715T00:24:49
   
   
   ----------------------------------------------------------------------
   Ran 93 tests in 27.345s
   
   FAILED (failures=1)
   test test_xmlrpc failed

🤖 New build scheduled with the buildbot fleet by @FFY00 for commit 6c342df 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F147951%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

The test_xmlrpc failure on iOS only seems weird, I triggered a re-run.

Well, the test is still failing. It seems like the time.localtime mock is not being installed in xmlrpc.client, or is losing the side effect after one call, weird.

I am gonna sync with main, just to be sure this is not a known issue.

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@FFY00 After looking into the code for a bit, this is the best test refinement I have come up with:

def test_default(self):
    # Patch BOTH time and localtime inside the client module
    with mock.patch('xmlrpc.client.time.time') as time_mock, \
         mock.patch('xmlrpc.client.time.localtime') as localtime_mock:
        
        # 1. Force the 'current' time to be a specific float
        time_mock.return_value = 1373847889.0 
        
        # 2. Force the conversion to be your specific struct
        time_struct = time.struct_time([2013, 7, 15, 0, 24, 49, 0, 196, 0])
        localtime_mock.return_value = time_struct
        
        t = xmlrpclib.DateTime()
        
        self.assertEqual(str(t), "20130715T00:24:49")

I think this can be simplified as follows:

@mock.patch('xmlrpc.client.time.time', return_value=1373847889.0)
@mock.patch(
    'xmlrpc.client.time.localtime',
    return_value=time.struct_time([2013, 7, 15, 0, 24, 49, 0, 196, 0]),
)
def test_default(self, *_):
    t = xmlrpclib.DateTime()
    
    self.assertEqual(str(t), "20130715T00:24:49")

I didn't expect the Spanish Inquisition

Nobody expects the Spanish Inquisition!

@FFY00: please review the changes made to this pull request.

I'm adding backport labels, but take them off if you see any room for surprise in the implications of this change and we want to sell this as a feature

It looks like the macOS failure is an instance of #149156

@warsaw @brettcannon this also fixes the same double call bug with the new pep 829 implementation

I want to fix and test this differently in Python 3.15, but this looks pretty good for 3.14 and 3.13. Once I land that in main (and not backport), I'll get this in shape for those older branches.

Thanks for both finding and fixing this bug!

@warsaw @asottile

Anything left for this? Perhaps a mention of which pull request was actually merged with the similar changes. There has been a lot to keep track of since this was opened.

yeah this can be closed out -- superseded by #149799