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

Fix Bug0013415 and 0013423 #1772

Open
wants to merge 3 commits into
base: maintenance/gramps52
Choose a base branch
from

Conversation

CameronD73
Copy link

Three commits, to fix the sortval bug and corresponding unit test. These need to go together to allow tests to pass.
The 3rd component is a trivial improvement to warning message when date unit tests are skipped.

The is_valid() method for a Date simply checks the sortval for a zero value. However, if an empty date is passed to _calc_sort_value() it first  adjusts to 1 ALL y,m,d values that are zero.  Thus, an empty date would return a sortval for the year 1-1-1 as a valid date.
This change to the unit-test code date_test.py is required so that the previous commit for bug #0013415 can pass the test.
The problematic part of the test calculates an age for somebody with a birth date of "" (an empty string) and a death date given by a year, such as 1760.
The previous test code required it to report "greater than 110 years", which should not be a suitable response. The previous code also reported an age of negative 1999 years for a birth year of 2000 and a death date of the empty string.
The unit test code now tests against a reply of "unknown".
The test is skipped for locales other than en_US, but the message simply reported that the locale had to be English, which is baffling for anybody with an English locale  other than US.
@Nick-Hall Nick-Hall added the bug label Sep 9, 2024
@QuLogic
Copy link
Contributor

QuLogic commented Sep 9, 2024

Commits should be atomic; i.e., the first two commits are related and should be the same commit (though if this PR is squash-merged then that issue is moot.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants