Skip to content

lib/datetime: Add pytest unit tests for datetime_scan and datetime_format - #7871

Merged
echoix merged 3 commits into
OSGeo:mainfrom
jalpatel11:add-lib-datetime-ctypes-tests
Sep 2, 2026
Merged

echoix merged 3 commits into
OSGeo:mainfrom
jalpatel11:add-lib-datetime-ctypes-tests

Conversation

@jalpatel11

Copy link
Copy Markdown
Contributor

Add ctypes unit tests for lib/datetime

Summary

lib/datetime is the C library behind G_format_timestamp and G_scan_timestamp, and is used by the timestamp functionality across GRASS, including r.timestamp, v.timestamp, r3.timestamp, and the t.* modules.

As discussed in #7802, the library did not previously have a dedicated test suite. This PR adds direct unit tests for the C datetime API using the existing grass.lib.date ctypes bindings.

The tests call the C API directly. They do not invoke CLI tools, start a GRASS session, or depend on test fixtures. Since lib/datetime does not depend on G_gisinit or other GIS state, this keeps the tests at the unit-test level rather than turning them into integration tests.

The suite contains 107 tests across 10 test files, following the existing ctypes test naming convention used by lib_vector_rtree_ctypes_test.py.

Test Coverage

lib_datetime_ctypes_test.py

Covers datetime_scan and datetime_format, including:

  • Absolute and relative timestamps
  • Round-trip parsing and formatting
  • BC dates
  • Timezone offsets and boundary values
  • Fractional and microsecond-level precision
  • Invalid timestamp input
  • Rejection of 12-hour clock formats where the API expects 24-hour input

lib_datetime_type_ctypes_test.py

Covers:

  • datetime_set_type
  • datetime_check_type
  • Valid datetime types
  • Documented error codes for invalid types

lib_datetime_values_ctypes_test.py

Covers the field-level getters and setters for:

  • Year
  • Month
  • Day
  • Hour
  • Minute
  • Second
  • Fractional seconds

Day validation is also covered against leap-year boundaries rather than treating February as having a fixed number of days.

lib_datetime_calendar_ctypes_test.py

Covers:

  • datetime_is_leap_year
  • datetime_days_in_year
  • datetime_days_in_month
  • Leap-year behavior
  • The documented simplification that BC years are never treated as leap years

lib_datetime_sign_ctypes_test.py

Covers the sign and comparison helpers, including:

  • Sign handling
  • Copying datetime values
  • Equality checks
  • Range-related helpers

lib_datetime_timezone_ctypes_test.py

Covers:

  • Timezone validation
  • Getting and setting timezone values
  • Unsetting the timezone
  • UTC conversion
  • Boundary and offset handling

lib_datetime_increment_ctypes_test.py

Covers datetime_increment and its associated validation helpers, including incrementing values across relevant calendar boundaries.

lib_datetime_difference_ctypes_test.py

Covers datetime_difference across:

  • Day differences
  • Month differences
  • Timezone-adjusted timestamps
  • Cases where timezone offsets affect the resulting difference

lib_datetime_change_from_to_ctypes_test.py

Covers precision changes between datetime representations, including:

  • Truncating precision
  • Extending precision
  • Zero-filling when increasing precision

The more involved rounding modes are intentionally left for a follow-up rather than expanding the scope of this PR.

lib_datetime_error_ctypes_test.py

Covers the library's global error-state API and verifies the expected error handling behavior.

Existing Behavior Noted During Testing

While testing datetime_change_timezone and datetime_change_to_utc, I noticed that both functions correctly adjust the clock fields but do not update dt->tz, despite the function documentation stating that the timezone field should also be updated.

This does not currently affect the only caller, datetime_difference, because that caller discards the timezone field after the conversion.

I have not changed that behavior as part of this PR. Instead, the tests document the current behavior and include a comment explaining the discrepancy rather than treating it as an intentional API guarantee.

I think this would be better handled as a separate issue/PR so that the behavioral change can be reviewed independently.

Validation

I tested the suite against an actual compiled version of lib/datetime rather than relying only on source-level inspection.

For the verification:

  1. Compiled the lib/datetime/*.c sources into a shared library.
  2. Ran the project's actual ctypesgen tooling against the real headers to generate the grass.lib.date bindings.
  3. Ran the complete new test suite against that compiled library.
  4. Verified that all 107 tests pass.
  5. Ran formatting and lint checks using the pinned ruff version from .pre-commit-config.yaml (v0.15.17).

This confirms that the tests exercise the compiled C implementation through the same ctypes interface used by the project, rather than testing a Python reimplementation or mocked behavior.

AI Disclosure

I used AI assistance (Claude) while drafting and iterating on the tests, including helping verify test cases against a real compiled lib/datetime build.

I reviewed the generated tests and validation results myself and made the final decisions about coverage, expected behavior, and the scope of the changes.

@github-actions github-actions Bot added Python Related code is in Python libraries tests Related to Test Suite labels Aug 31, 2026
@jalpatel11

Copy link
Copy Markdown
Contributor Author

@echoix, Can you review the PR?

@jalpatel11

Copy link
Copy Markdown
Contributor Author

@ninsbl review?

@ninsbl ninsbl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Really comprehensive tests. Thanks for the effort.

They look good to me. Only thing to consider is whether we want to add so much test code for a rather small part of the code-base (datetime only).. It does not seem to add noticable time to the pytest execution though, so that part seems fine too...

I leave that for other maintainers to decide and merge (or request changes)...

@echoix echoix left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The time taken doesn't seem so bad (all in the same second). I'm surprised the ctypes didn't need a subprocess to work, like discussed in #7862, but I guess it's fine as is.

@echoix
echoix merged commit c0f7eab into OSGeo:main Sep 2, 2026
26 checks passed
@github-actions github-actions Bot added this to the 8.6.0 milestone Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libraries Python Related code is in Python tests Related to Test Suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants