Opened 9 months ago

Closed 8 months ago

#29922 closed defect (fixed)

util/time test failure on Jenkins

Reported by: ahf Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version: Tor:
Severity: Normal Keywords: 040-must, 035-backport, 040-backport, tor-ci-fail
Cc: Actual Points: 0.5
Parent ID: Points: 1
Reviewer: teor Sponsor: Sponsor31-can


Our 'util/time' test fails on our tor-ci-mingwcross-master-test builder with:

util/time: Mar 27 13:29:43.773 [warn] tor_gmtime_r(): Bug: gmtime(-1) failed with error Invalid argument: Rounding up to 1970 (on Tor )
   [time FAILED]


Child Tickets

Change History (13)

comment:1 Changed 9 months ago by teor

Keywords: 040-must added
Owner: set to teor
Points: 1
Status: newassigned

I think this failure might be caused by the "fail tests on bugs and errors" fix.
So we need to fix these bug warnings somehow.

We must have fixed them on master on other platforms, and in Appveyor. So I wonder why the fix isn't working in jenkins?

I'll try to look into this tomorrow (~18 hours).

comment:2 Changed 9 months ago by nickm

It's possible that this is a portability issue -- I have noticed that different Windows implementations tend to give different libc timeval manipulation functions.

comment:3 Changed 9 months ago by teor

Keywords: tor-ci-fail added

comment:4 Changed 9 months ago by teor

Keywords: 035-backport 040-backport added

We will have to backport to 0.3.5, because that's where we started failing on errors.

comment:5 Changed 8 months ago by teor

Actual Points: 0.2
Version: Tor: unspecifiedTor:

Hi Nick, I tried to fix this issue, but it's hard without a backtrace on Windows. I couldn't really work out where to start.

Do you think we should:

  • downgrade the bugs to warnings in src/lib/encoding/time_fmt.c
  • fix the util/time unit test?
  • handle the bugs in the util/time unit test?

Or something else?

comment:6 Changed 8 months ago by teor

Owner: changed from teor to nickm

comment:7 Changed 8 months ago by nickm

Actual Points: 0.20.3
Reviewer: teor
Status: assignedneeds_review

Likely fix in my branch bug29922_035 with PR at .

I won't be absolutely sure till we merge it, but it seems okay to me so far. Setting teor as reviewer in hopes we can merge this week.

comment:8 Changed 8 months ago by teor

Actual Points: 0.30.4
Keywords: asn-merge added
Sponsor: Sponsor31-can
Status: needs_reviewmerge_ready

If we tried really hard, we might be able to squeeze this into Sponsor 31's modularity support, because it came from our unit test error-handling impovements. Better tests are important for modularity.

Looks fine to me.

comment:9 Changed 8 months ago by asn

Keywords: asn-merge removed

merged to 040 and forward. leaving it open for backports.

comment:10 Changed 8 months ago by nickm

Milestone: Tor: 0.4.0.x-finalTor: 0.3.5.x-final

comment:11 Changed 8 months ago by nickm

Appears not to have worked on jenkins: it was looking for the wrong error. I added another fix here and merged it to 0.4.0 and later.

comment:12 Changed 8 months ago by teor

It looks like this worked on jenkins, I'll merge to 0.3.5 so we can get binaries built for Windows again.

comment:13 Changed 8 months ago by teor

Actual Points: 0.40.5
Resolution: fixed
Status: merge_readyclosed

Merged to 0.3.5 and merged forward.

Merged #29922 with #30041.

Note: See TracTickets for help on using tickets.