Fix test generation for data updates #1059
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `-V` flag was introduced 10 years ago but macOS uses a very old version of `zdump` that doesn't support it. The versions of `zic` and `zdump` shipped with macOS only support 32-bit data files. A while ago the `data-zdump` task was changed from `-V` to `-v` to support building on macOS, but that also switched to a far more verbose output format on Linux machines. The best solution is to use `-v` for macOS but `-V` for newer versions of `zdump`. This also logs some warnings when `zic` and `zdump` are outdated. The best solution for building the data on macOS currently is to manually compile `zic` and `zdump` from the tzcode source.
This allows people on macOS (or other systems shipping old tzcode versions) to compile `zic` and `zdump` manually to a different location, then use them in the data compilation pipeline. Usage: - `grunt data-zic --zic-path=/path/to/custom/zic` - `grunt data-zdump --zdump-path=/path/to/custom/zdump`
The old data-tests behaviour skipped the first 2 and last 2 lines of the `zdump` output when generating transition tests. This was done when the build process only used the 32-bit `zdump` output. The output always included transitions for the 32-bit boundaries (1901 and 2038), regardless of the actual zone data. Now that the build uses 64-bit data by default, this behaviour was skipping out valid transition tests. This commit removes the skipping step, but adds an extra check for fixed zones with no transitions. Zones like `Etc/UTC` have no transitions, so weren't generating any transition tests. Now they generate a single test for 1970-01-01.
This was referenced May 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a few things that were bugging me about the release process.
zdumpoutput and test generation code was all written for an old 32-bit version ofzdump. But the newer 64-bit versions have different flags and output. This resulted in an overly-verbose output format, and accidentally skipping the first and last transitions when generating unit tests. (Or in the case of fixed-offset zones, generating no tests at all.)zicandzdumpin macOS are horrendously old and only support 32-bit data. I've been manually compiling the binaries from tzdb instead, then temporarily changingdata-zic.jsanddata-zdump.jsto reference the custom versions.countriesunit tests weren't auto-generated, so we have to keep manually tweaking them when data changes during a release. This is error-prone and inconsistent (e.g. it turns out some new zones were missing from the test file).So after a fair amount of comparing the output of
zdumpon various machines, this PR does the following:zicandzdumpbinaries by passing--zic-path=/path/to/zicand--zdump-path=/path/to/zdumparguments to thegrunt datatasks.zicorzdumpdoesn't support 64-bit data, and switch between the-Vand-vflags depending on what's supported.zdumpoutput validation and comments.zone.countries()andtz.zonesForCountry().