Skip to content

ci: move main build to use test-rust nox job#5091

Merged
davidhewitt merged 2 commits intomainfrom
nox-main-build
May 9, 2025
Merged

ci: move main build to use test-rust nox job#5091
davidhewitt merged 2 commits intomainfrom
nox-main-build

Conversation

@davidhewitt
Copy link
Member

As a precursor to investigating #5080, I wanted to make the test workflow in CI more similar to what I can run locally.

This modifies the noxfile's test-rust job to be much closer to build.yml semantics, and then changes build.yml to call it.

@davidhewitt davidhewitt added CI-build-full CI-no-fail-fast If one job fails, allow the rest to keep testing labels Apr 25, 2025
@davidhewitt
Copy link
Member Author

Hmm, @bschoenmaeckers got any idea what the jiff failure might be caused by?

  ---- src/conversions/jiff.rs - conversions::jiff (line 22) stdout ----
  Test executable failed (exit status: 1).
  
  stderr:
  Error: PyErr { type: <class 'ValueError'>, value: ValueError('cannot convert non-fixed IANA time zone to offset without timestamp or civil datetime'), traceback: None }
  
  
  
  failures:
      src/conversions/jiff.rs - conversions::jiff (line 22)

(PyPy and GraalPy failures are unrelated, I'll look into those separately.)

@bschoenmaeckers
Copy link
Member

This is maybe related to #5055.

I disabled the zoneinfo branch on pre python 3.9. It looks like it tries to convert a non-fixed timezone to a fixed timezone which fails. Before this it would throw a import error.

@davidhewitt
Copy link
Member Author

Makes sense, I think we can avoid this by converting timezone to UTC before trying the to-Python conversion (which might be a good idea for test reliability anyway).

@bschoenmaeckers
Copy link
Member

Can we conditional test this on 3.9+? So it is able to convert the timezone correctly?

@bschoenmaeckers
Copy link
Member

bschoenmaeckers commented May 7, 2025

An other option is conditional on 3.8 to convert the timezone into a fixed timezone before converting using Timezone::fixed. This way it is able to convert a 'Zoned' just fine, but without proper timezone info.

@davidhewitt
Copy link
Member Author

Thanks for pushing that PR. Given that this seems to fix some cases worth resolving before the 0.25 release, I'm going to merge this as-is with the .in_tz("UTC") bit, and push a follow-up PR to remove that again.

@davidhewitt davidhewitt enabled auto-merge May 9, 2025 11:20
@davidhewitt davidhewitt added this pull request to the merge queue May 9, 2025
Merged via the queue into main with commit 260089a May 9, 2025
87 of 93 checks passed
@davidhewitt davidhewitt deleted the nox-main-build branch May 9, 2025 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI-build-full CI-no-fail-fast If one job fails, allow the rest to keep testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants