onsi / ginkgo

A Modern Testing Framework for Go
http://onsi.github.io/ginkgo/
MIT License
8.38k stars 660 forks source link

time formats are US specific #1439

Open plastikfan opened 3 months ago

plastikfan commented 3 months ago

This is a minor issue, but if you are used to time formats other than US, this can become a little grating, eg:

• [FAILED] [0.019 seconds]
Sampling sample [It] 🧪 ===> given: 'universal(filter): first, single file, first 2 folders', should: 'invoke for at most single file per directory'
/Users/plastikfan/dev/github/snivilised/traverse/internal/kernel/navigator-sample_test.go:179

  Timeline >>
  ---> 🌊 SAMPLE-CALLBACK: '/Users/plastikfan/dev/github/snivilised/traverse/test/data/MUSICO/edm'
  [FAILED] in [It] - /Users/plastikfan/dev/github/snivilised/traverse/internal/kernel/kernel-suite_test.go:291 @ 07/30/24 09:38:45.261
  << Timeline

  [FAILED] ❌ incorrect no of items for: 'Incorrect no of files', expected: '7', actual: '0'
  Expected
      <uint>: 0
  to equal
      <uint>: 7
  In [It] at: /Users/plastikfan/dev/github/snivilised/traverse/internal/kernel/kernel-suite_test.go:291 @ 07/30/24 09:38:45.261

The time displayed as '07/30/24 09:38:45.261' is a bit irritating. I'm not particluarly against seeing date/times in US format. The irritating part is the ambiguity. Now I know the example date I've used here is not ambiguous, but August is upon us and we'll be back to ambigupus dates that makes this issue more acute.

I wouldn't be averse to seeing a date shown in US format Jul 30 2024 (non ambiguous), but it would be great if either a universal time format was used or the time display was related to the user's locale or indeed could be controlled by the user via an environment variable.

I noticed that GINKGO_TIME_FORMAT defined in ginkgo/types is defined as:

const GINKGO_TIME_FORMAT = "01/02/06 15:04:05.999"

so the quickess solution would be just to change this to reference one of the predefined time formats in the time package, but the better solution would be create the format from the locale or via environment variable override.

plastikfan commented 3 months ago

... actually, changing GINKGO_TIME_FORMAT, doesnt resolve the issue so I don't know what the real fix is.

onsi commented 3 months ago

heyo sorry for the delay. Changing GINKGO_TIME_FORMAT does work, however you'll need to recompile the ginkgo cli.

I have a quick fix that will let you set a GINKGO_TIME_FORMAT environment variable and it'll get picked up (no need to recompile, etc.). I'd rather not change what ships out of the box as there may be parsers out that that expect the current format. (Funny how these things become part of the implicit external contract of the codebase). Thoughts?

plastikfan commented 2 months ago

Hi @onsi , yeah you have a point about the impact that chaning the format might have on downstream services and I would never expect you to release a breaking change. I do like the idea of the environment variable, that would be great.

Actually when I tested modifying GINKGO_TIME_FORMAT internal variable, I did recompile and re-deploy the cli locally, but it seemed to have no effect, so somehow I probably did something wrong. (Actually, this was really odd, because I changed the content of the ginko message just to be sure that my own compiled version was being run and that change was in effect, but my modified version of GINKGO_TIME_FORMAT still had no effect!).

PS: no worries about the delay, I never expect immediate responses as you're running a well respected, well used and a great project and I know you must be busy juggling many interests.

onsi commented 3 weeks ago

I've added support for GINKGO_TIME_FORMAT as an env variable and will cut a release soon.