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

Jump to bottom

Add tests for the datetime C API #80963

Closed
pganssle opened this issue May 3, 2019 · 9 comments
Closed

Add tests for the datetime C API #80963

pganssle opened this issue May 3, 2019 · 9 comments
Assignees
@Mariatta
Labels
3.8 only security fixes easy tests Tests in the Lib/test dir type-feature A feature request or enhancement

Comments

@pganssle
Copy link
Member

pganssle commented May 3, 2019

BPO 36782
Nosy @vstinner, @ericvw, @Mariatta, @pganssle, @tonybaloney, @SimiCode
PRs
  • bpo-36782: Created C API wrappers and added missing tests for functions in the PyDateTimeAPI. #13088
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/Mariatta'
closed_at = <Date 2019-05-27.23:36:05.321>
created_at = <Date 2019-05-03.13:46:57.506>
labels = ['easy', '3.8', 'type-feature', 'tests']
title = 'Add tests for the datetime C API'
updated_at = <Date 2019-05-27.23:36:05.316>
user = 'https://github.com/pganssle'

    bugs.python.org fields:

    activity = <Date 2019-05-27.23:36:05.316>
actor = 'vstinner'
assignee = 'Mariatta'
closed = True
closed_date = <Date 2019-05-27.23:36:05.321>
closer = 'vstinner'
components = ['Tests']
creation = <Date 2019-05-03.13:46:57.506>
creator = 'p-ganssle'
dependencies = []
files = []
hgrepos = []
issue_num = 36782
keywords = ['patch', 'easy']
message_count = 9.0
messages = ['341336', '341346', '341400', '341408', '341497', '341673', '341674', '342761', '343705']
nosy_count = 6.0
nosy_names = ['vstinner', 'ericvw', 'Mariatta', 'p-ganssle', 'anthony shaw', 'edison.abahurire']
pr_nums = ['13088']
priority = 'low'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue36782'
versions = ['Python 3.8']
    @pganssle
    Copy link
    Member Author

    pganssle commented May 3, 2019

    A decent fraction of the datetime C API has no tests. If we had tests, we could have prevented bpo bpo-36025, which was a regression in the FromTimestamp method.

    I would like to open this issue to suggest the addition of tests for the full interface, to prevent further regressions here.

    To write a test, first add a wrapper function in the _testcapimodule module: https://github.com/python/cpython/blob/master/Modules/_testcapimodule.c#L2218

    Then test that function here: https://github.com/python/cpython/blob/master/Lib/test/datetimetester.py#L5821

    See an example at GH PR 11922: #11922

    I recommend testing *both* the version of the function accessed by the Macro (e.g. PyDate_FromTimestamp) and from the C API capsule (e.g. PyDateTimeApi->FromTimestamp), since projects access these functions in both ways.

    C API Documentation is here: https://docs.python.org/3/c-api/datetime.html

    Currently untested:

    • PyDate_FromDate / PyDateTimeAPI->Date_FromDate
    • PyDateTime_FromDateAndTime / PyDateTimeAPI->DateTime_FromDateAndTime
    • PyDateTime_FromDateAndTimeAndFold / PyDateTimeAPI->DateTime_FromDateAndTimeAndFold
    • PyTime_FromTime -> PyDateTimeAPI->Time_FromTime
    • PyTime_FromTimeAndFold -> PyDateTime->Time_FromTimeAndFold
    • PyDelta_FromDSU / PyDateTime->Delta_FromDelta

    Untested macros with no corresponding API module:

    • PyDateTime_DATE_GET_YEAR

    • PyDateTime_DATE_GET_MONTH

    • PyDateTime_DATE_GET_DAY

    • PyDateTime_DATE_GET_HOUR

    • PyDateTime_DATE_GET_MINUTE

    • PyDateTime_DATE_GET_SECOND

    • PyDateTime_DATE_GET_MICROSECOND

    • PyDateTime_DELTA_GET_DAYS

    • PyDateTime_DELTA_GET_SECONDS

    • PyDateTime_DELTA_GET_MICROSECONDS

    I can spawn smaller issues for this if that's preferred, but I figured we'd start with this big meta-issue.

    @pganssle pganssle added 3.9 only security fixes 3.8 only security fixes tests Tests in the Lib/test dir easy type-feature A feature request or enhancement labels May 3, 2019
    @pganssle
    Copy link
    Member Author

    pganssle commented May 3, 2019

    This ticket should be reserved for the mentored sprint.

    @tonybaloney
    Copy link
    Mannequin

    tonybaloney mannequin commented May 4, 2019

    Reserving this issue (in mentored sprint)

    @simicode
    Copy link
    Mannequin

    simicode mannequin commented May 4, 2019

    I have done PyDate_FromDate in #57297

    @simicode
    Copy link
    Mannequin

    simicode mannequin commented May 6, 2019

    I have done PyDateTime_FromDateAndTime. I'm moving on to add PyDateTime_FromDateAndTimeAndFold (plus documentation for fold as requested in https://bugs.python.org/issue36783) before making a PR.

    @simicode
    Copy link
    Mannequin

    simicode mannequin commented May 7, 2019

    I have submitted PR (under review) for these tests:

    • PyDate_FromDate / PyDateTimeAPI->Date_FromDate
    • PyDateTime_FromDateAndTime / PyDateTimeAPI->DateTime_FromDateAndTime
    • PyDateTime_FromDateAndTimeAndFold / PyDateTimeAPI->DateTime_FromDateAndTimeAndFold
    • PyTime_FromTime -> PyDateTimeAPI->Time_FromTime
    • PyTime_FromTimeAndFold -> PyDateTime->Time_FromTimeAndFold
    • PyDelta_FromDSU / PyDateTime->Delta_FromDelta

    @simicode
    Copy link
    Mannequin

    simicode mannequin commented May 7, 2019

    @p-ganssle
    Please open a new bpo for the Untested macros. I will be happy to work on that in the sprints tomorrow.

    @vstinner
    Copy link
    Member

    New changeset 98ff4d5 by Victor Stinner (Edison A) in branch 'master':
    bpo-36782: Created C API wrappers and added missing tests for functions in the PyDateTimeAPI. (bpo-13088)
    98ff4d5

    @vstinner
    Copy link
    Member

    Paul Ganssle is in favor of backporting new tests to Python 3.7:
    #13088 (comment)

    But automated backports failed with a conflict. If someone wants to backport it manually, please go ahead!

    In the meanwhile, I close the issue.

    @vstinner vstinner removed the 3.9 only security fixes label May 27, 2019
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @Mariatta Mariatta

    Labels
    3.8 only security fixes easy tests Tests in the Lib/test dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @vstinner @pganssle @Mariatta