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

datetime test failure: "Undefined TimeZone: America/New_York" #125

Closed
mmuetzel opened this issue Feb 8, 2024 · 6 comments
Closed

datetime test failure: "Undefined TimeZone: America/New_York" #125

mmuetzel opened this issue Feb 8, 2024 · 6 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@mmuetzel
Copy link

mmuetzel commented Feb 8, 2024

pkg test is failing for @datetime/datetime.m with the following error:

  >>>>> processing /usr/share/octave/packages/tablicious-0.4.2/@datetime/datetime.m
  ***** test datetime ('2011-03-07 12:34:56', 'TimeZone', 'America/New_York');
  !!!!! test failed
  Undefined TimeZone: America/New_York
  ***** test
    d = datetime;
    d.TimeZone = 'America/New_York';
    d2 = d;
    d2.TimeZone = 'America/Chicago';
    assert (abs (datenum (d) - datenum (d2)), (1/24), .0001)
  !!!!! test failed
  Undefined TimeZone: America/New_York

Not sure what that means.

See the CI tests over at gnu-octave/packages#401


Andrew's notes

Turns out this is what happens when you run on a Linux box that doesn't have tzdb installed (package tzdata on Ubuntu). We can at least provide a better error message here.

Handling that under separate ticket #128.

@apjanke
Copy link
Owner

apjanke commented Feb 8, 2024

Undefined TimeZone: America/New_York

Hmm. That looks like it's not finding the time zone data in its tzdb, and producing a not-great error message. On Mac and Linux, Tablicious uses the system tzdb provided by the OS, not an internal database bundled with the package.

What OS and distro is this on? Maybe I'm making a bad assumption about where the tzdb data is stored on some Linux distros, and that's showing up as it not finding the particular tzdb file when it's looking at the wrong path, instead of it noticing the entire tzdb database is not located where expected.

@apjanke apjanke changed the title pkg test failing for @datetime/datetime.m datetime test failure: "Undefined TimeZone: America/New_York" Feb 8, 2024
@apjanke apjanke self-assigned this Feb 9, 2024
@apjanke apjanke added the bug Something isn't working label Feb 9, 2024
@apjanke apjanke moved this from Needs triage to High priority in Octave-Tablicious Feb 9, 2024
@github-project-automation github-project-automation bot moved this to Needs triage in Octave-Tablicious Feb 9, 2024
@apjanke apjanke added this to the 0.4.4 milestone Feb 9, 2024
@mmuetzel
Copy link
Author

mmuetzel commented Feb 9, 2024

What OS and distro is this on?

Afaict, the tests on octave/packages are run on an Ubuntu based docker image built with the rules from https://github.com/gnu-octave/docker.
Do they need to install some package for that database to be available?

Fwiw, I'm seeing the following with the tablicious package (currently still version 0.3.7) using the 1st release candidate of Octave 9 on Windows:

Testing functions in package 'tablicious':

Integrated test scripts:

                                                                                  [ CPU    /  CLOCK ]
  ...90\mingw64\share\octave\packages\tablicious-0.3.7\datetime.m  pass    2/4    [ 0.609s /  1.183s]
                                                                   FAIL    2
  ...90\mingw64\share\octave\packages\tablicious-0.3.7\duration.m  pass    3/3    [ 0.016s /  0.047s]

Fixed test scripts:


Failure Summary:

  ...90\mingw64\share\octave\packages\tablicious-0.3.7\datetime.m  pass    2/4
                                                                   FAIL    2

Summary:

  PASS                                5
  FAIL                                2

See the file C:\Users\Markus\fntests.log for additional details.

270 (of 272) .m files have no tests.

Please help improve Octave by contributing tests for these files
(see the list in the file C:\Users\Markus\fntests.log).

Respective part of fntests.log:

>>>>> processing C:\Program Files\GNU Octave\Octave-9.0.90\mingw64\share\octave\packages\tablicious-0.3.7\datetime.m
***** test datetime ('2011-03-07 12:34:56', 'TimeZone','America/New_York');
!!!!! test failed
POSIX zone rules are unimplemented
***** test
  d = datetime;
  d.TimeZone = 'America/New_York';
  d2 = d;
  d2.TimeZone = 'America/Chicago';
  assert (abs(d.dnums - d2.dnums), (1/24), .0001)
!!!!! test failed
ASSERT errors for:  assert (abs (d.dnums - d2.dnums),(1 / 24),.0001)

  Location  |  Observed  |  Expected  |  Reason
     ()           0         0.041667     Abs err 0.041667 exceeds tol 0.0001 by 0.04
>>>>> processing C:\Program Files\GNU Octave\Octave-9.0.90\mingw64\share\octave\packages\tablicious-0.3.7\duration.m

Maybe, it is still missing the same dependency? Or a different issue?

@mmuetzel
Copy link
Author

mmuetzel commented Feb 9, 2024

Do they need to install some package for that database to be available?

To answer my own question: The test passes after installing the Ubuntu package tzdata.

@apjanke
Copy link
Owner

apjanke commented Feb 9, 2024

To answer my own question: The test passes after installing the Ubuntu package tzdata.

Ah, cool. That'll do it. On Ubuntu, those tzdb files are from package tzdata. On my Ubuntu, they're at the expected /usr/share/zoneinfo location

janke@ubix:/usr/share/zoneinfo$ cat /etc/lsb-release 
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=22.04
DISTRIB_CODENAME=jammy
DISTRIB_DESCRIPTION="Ubuntu 22.04.3 LTS"
janke@ubix:/usr/share/zoneinfo$ dpkg -S /usr/share/zoneinfo/America/New_York 
tzdata: /usr/share/zoneinfo/America/New_York
janke@ubix:/usr/share/zoneinfo$ 

I'm kinda surprised that the Ubuntu Docker image would have been missing the tzdata package in the first place. It's such a basic-seeming package that I would have expected it to be in even a minimal Linux image. But maybe minimal Linux images are really minimal, for use on stuff like embedded systems, or just super-lightweight containers.

I (and thus Tablicious) had assumed that tzdata would be present on any Linux system, and thus searches pretty unconditionally. That's not great, as we saw here.

A couple things I can do:

A) Do more sophisticated checking to see if the tzdb database as a whole is absent, as opposed to a specific timezone file being absent which indicates an invalid timezone identifier, and raise a better error message in that case. That would surely be good.

B) On Linux systems that are missing the tzdb db at the system level, Tablicious could fall back to using the bundled tzdb it ships with, like it does on Windows. That would get it to run in oddball locations like this. But I'm not sure if it's a great idea: it would then be using different code paths, and the tzdb it ships with may be stale, so that might lead to slightly different results, affecting test runners like this, and maybe oddball users who lack tzdb. Have to think about this one for a bit before doing it.

For option A, I'll put that together. This'll be a learning experience figuring out how to build an Ubuntu VM that doesn't have tzdata for testing it – even the ubuntu-minimal package requires tzdata, it seems.

janke@ubix:/usr/share/zoneinfo$ apt-cache rdepends --installed tzdata
tzdata
Reverse Depends:
  ubuntu-minimal
  python3-tz
  libmozjs-91-0
  ubuntu-minimal
  python3-tz
  python3-dateutil
  libtcl8.6
  libmozjs-91-0
  libical3

Guess I should just learn Docker and build off the version of your base image that lacked tzdata in the first place?

@mmuetzel
Copy link
Author

mmuetzel commented Feb 9, 2024

Maybe, the Octave Docker image is really the odd ball out. No idea why it doesn't come with that package pre-installed.
I don't know much about Docker either. So unfortunately, I can't help with setting it up.

Fwiw, I updated to the latest version of tablicious on Windows (using the release candidate Octave 9.0.90). All tests are passing. 🎉

>> pkg install -forge tablicious
>> pkg test tablicious
Testing functions in package 'tablicious':

Integrated test scripts:

                                                                                  [ CPU    /  CLOCK ]
  ..octave\api-v58\packages\tablicious-0.4.2\@datetime\datetime.m  pass    4/4    [ 0.875s /  1.173s]
  ..octave\api-v58\packages\tablicious-0.4.2\@duration\duration.m  pass    3/3    [ 0.047s /  0.040s]
  ..ing\octave\api-v58\packages\tablicious-0.4.2\@string\string.m  pass    6/6    [ 0.062s /  0.085s]
  ..ppData\Roaming\octave\api-v58\packages\tablicious-0.4.2\NaS.m  pass    1/1    [ 0.000s /  0.005s]
  ..ta\Roaming\octave\api-v58\packages\tablicious-0.4.2\missing.m  pass    2/2    [ 0.031s /  0.020s]

Fixed test scripts:


Summary:

  PASS                               16
  FAIL                                0

See the file C:\Users\Markus\fntests.log for additional details.

286 (of 291) .m files have no tests.

Please help improve Octave by contributing tests for these files
(see the list in the file C:\Users\Markus\fntests.log).

>>

I'd be ok with closing this as invalid if you agree.

@apjanke
Copy link
Owner

apjanke commented Feb 9, 2024

Yeah, let's close this. Seems like quite an unusual case, so not urgent to handle, and the immediate problem is fixed.

I'll look at a long-term improvement for this under separate low-priority ticket #128, and close this one out to keep you from getting spammed with comment notifications you don't need to worry about.

@apjanke apjanke closed this as completed Feb 9, 2024
@github-project-automation github-project-automation bot moved this from High priority to Closed in Octave-Tablicious Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Closed
Development

No branches or pull requests

2 participants