Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Prepare unit tests for Python 3.12 #16099

Merged
merged 7 commits into from
Aug 25, 2023
Merged

Prepare unit tests for Python 3.12 #16099

merged 7 commits into from
Aug 25, 2023

Conversation

V02460
Copy link
Contributor

@V02460 V02460 commented Aug 10, 2023

Fixes errors of trial tests running under Python3.12 by

  • extend TerseJsonFormatter’s ignorelist of LogRecord properties by the taskName property. (Please double check this is correct!)
  • replacing the deprecated assertDictContainsSubset with assertLessEqual, arguably improving error messages and readability of code.

Note: After applying this PR, there is one broken test left, which is tests.handlers.test_user_directory.UserDirectoryTestCase.test_handle_user_deactivated_support_user. It has a comment describing a bug:

synapse/tests/handlers/test_user_directory.py

Lines 461 to 463 in 7f4b413

# BUG: the correct spelling is assert_not_called, but that makes the test fail
# and it's not clear that this is actually the behaviour we want.
mock_remove_from_user_dir.not_called()

Replacing the now unknown not_called with assert_not_called gives a call count of one, instead of zero. Is the program logic being tested here correct?

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Pull request includes a sign off
  • Code style is correct
    (run the linters)

@V02460 V02460 requested a review from a team as a code owner August 10, 2023 22:24
@clokep
Copy link
Member

clokep commented Aug 11, 2023

For reference: https://docs.python.org/3.12/whatsnew/3.12.html#removed

clokep
clokep reviewed Aug 11, 2023
View reviewed changes
changelog.d/16099.bugfix Outdated Show resolved Hide resolved
changelog.d/16099.bugfix Outdated Show resolved Hide resolved
tests/rest/client/test_relations.py Outdated Show resolved Hide resolved
@V02460
Copy link
Contributor Author

V02460 commented Aug 19, 2023

Rebased because of conflicts.

@V02460 V02460 force-pushed the fix-tests-py3.12 branch 2 times, most recently from 34394a8 to dd87ea5 Compare August 21, 2023 16:00
@clokep
Copy link
Member

clokep commented Aug 21, 2023

Looks like CI worked, but there's a few more errors!

@V02460
Copy link
Contributor Author

V02460 commented Aug 21, 2023

Yeah, one of the errors is the one I described above. I changed tests.handlers.test_user_directory.UserDirectoryTestCase.test_handle_user_deactivated_support_user to expose the “real” error behind it. Can you tell me what the desired behavior for this test is, aka whether to change Synapse’s behavior or the test?

For the other error, I don’t get that one for the Fedora builds, but I’ll look into it.

@clokep
Copy link
Member

clokep commented Aug 22, 2023

Can you tell me what the desired behavior for this test is, aka whether to change Synapse’s behavior or the test?

I think #16157 will fix this.

@V02460
Copy link
Contributor Author

V02460 commented Aug 22, 2023

Trial completes successfully now after your PR and bumping the xmlschema version. Some others tests fail now, but I don’t understand from the error messages what is causing it.

@clokep
Copy link
Member

clokep commented Aug 24, 2023

bumping the xmlschema version

Any reason why this is needed? Shouldn't the proper version for python 3.12 get installed?

Some others tests fail now, but I don’t understand from the error messages what is causing it.

I think there was some networking errors and flakes, I've re-run them for now.

@V02460
Copy link
Contributor Author

V02460 commented Aug 24, 2023

Any reason why this is needed? Shouldn't the proper version for python 3.12 get installed?

I’m not sure I understand. Current xmlschema in poetry.lock is at v2.2.2, only for v2.2.3 xmlschema’s release notes claim added support for Python 3.12. Because xmlschema is only a transitive dependency, I figured poetry update xmlschema would be the proper thing to do. Maybe I am missing a way how to discriminate dependency versions based on the Python version?

I think there was some networking errors and flakes, I've re-run them for now.

Thanks! :) Still one test dying on a timeout after the re-run, though.

clokep
clokep approved these changes Aug 25, 2023
View reviewed changes
Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👍

@clokep
Copy link
Member

clokep commented Aug 25, 2023

The error here was a known flake and this is (mostly) changing unit tests. I think this is fine to merge.

@clokep clokep merged commit 84f441f into matrix-org:develop Aug 25, 2023
hughns pushed a commit that referenced this pull request Sep 4, 2023
@V02460 @hughns
Prepare unit tests for Python 3.12 (#16099)
f86d9a8
DMRobertson pushed a commit that referenced this pull request Sep 5, 2023
Merge tag 'v1.92.0rc1' into develop
02bc590 
- Add configuration setting for CAS protocol version. Contributed by Aurélien Grimpard. ([\#15816](#15816))
- Suppress notifications from message edits per [MSC3958](matrix-org/matrix-spec-proposals#3958). ([\#16113](#16113))
- Return a `Retry-After` with `M_LIMIT_EXCEEDED` error responses. ([\#16136](#16136))
- Add `last_seen_ts` to the [admin users API](https://matrix-org.github.io/synapse/latest/admin_api/user_admin_api.html). ([\#16218](#16218))
- Improve resource usage when sending data to a large number of remote hosts that are marked as "down". ([\#16223](#16223))

- Fix IPv6-related bugs on SMTP settings, adding groundwork to fix similar issues. Contributed by @evilham and @telmich (ungleich.ch). ([\#16155](#16155))
- Fix a spec compliance issue where requests to the `/publicRooms` federation API would specify `include_all_networks` as a string. ([\#16185](#16185))
- Fix inaccurate error message while attempting to ban or unban a user with the same or higher PL by spliting the conditional statements. Contributed by @leviosacz. ([\#16205](#16205))
- Fix a rare bug that broke looping calls, which could lead to e.g. linearly increasing memory usage. Introduced in v1.90.0. ([\#16210](#16210))
- Fix a long-standing bug where uploading images would fail if we could not generate thumbnails for them. ([\#16211](#16211))
- Fix a long-standing bug where we did not correctly back off from servers that had "gone" if they returned 4xx series error codes. ([\#16221](#16221))

- Update links to the [matrix.org blog](https://matrix.org/blog/). ([\#16008](#16008))
- Document which [admin APIs](https://matrix-org.github.io/synapse/latest/usage/administration/admin_api/index.html) are disabled when experimental [MSC3861](matrix-org/matrix-spec-proposals#3861) support is enabled. ([\#16168](#16168))
- Document [`exclude_rooms_from_sync`](https://matrix-org.github.io/synapse/v1.92/usage/configuration/config_documentation.html#exclude_rooms_from_sync) configuration option. ([\#16178](#16178))

- Prepare unit tests for Python 3.12. ([\#16099](#16099))
- Fix nightly CI jobs. ([\#16121](#16121), [\#16213](#16213))
- Describe which rate limiter was hit in logs. ([\#16135](#16135))
- Simplify presence code when using workers. ([\#16170](#16170))
- Track per-device information in the presence code. ([\#16171](#16171), [\#16172](#16172))
- Stop using the `event_txn_id` table. ([\#16175](#16175))
- Use `AsyncMock` instead of custom code. ([\#16179](#16179), [\#16180](#16180))
- Improve error reporting of invalid data passed to `/_matrix/key/v2/query`. ([\#16183](#16183))
- Task scheduler: add replication notify for new task to launch ASAP. ([\#16184](#16184))
- Improve type hints. ([\#16186](#16186), [\#16188](#16188), [\#16201](#16201))
- Bump black version to 23.7.0. ([\#16187](#16187))
- Log the details of background update failures. ([\#16212](#16212))
- Cache device resync requests over replication. ([\#16241](#16241))

* Bump anyhow from 1.0.72 to 1.0.75. ([\#16141](#16141))
* Bump furo from 2023.7.26 to 2023.8.19. ([\#16238](#16238))
* Bump phonenumbers from 8.13.18 to 8.13.19. ([\#16237](#16237))
* Bump psycopg2 from 2.9.6 to 2.9.7. ([\#16196](#16196))
* Bump regex from 1.9.3 to 1.9.4. ([\#16195](#16195))
* Bump ruff from 0.0.277 to 0.0.286. ([\#16198](#16198))
* Bump sentry-sdk from 1.29.2 to 1.30.0. ([\#16236](#16236))
* Bump serde from 1.0.184 to 1.0.188. ([\#16194](#16194))
* Bump serde_json from 1.0.104 to 1.0.105. ([\#16140](#16140))
* Bump types-psycopg2 from 2.9.21.10 to 2.9.21.11. ([\#16200](#16200))
* Bump types-pyyaml from 6.0.12.10 to 6.0.12.11. ([\#16199](#16199))
@DMRobertson DMRobertson mentioned this pull request Oct 17, 2023
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@clokep clokep clokep approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@V02460 @clokep