Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 2.7 tests #2145

Merged
merged 41 commits into from
Jun 7, 2023
Merged

Fix 2.7 tests #2145

merged 41 commits into from
Jun 7, 2023

Conversation

sentrivana
Copy link
Contributor

@sentrivana sentrivana commented May 26, 2023

The Python 2.7 common test suite fails to execute properly, leading the CI to think the tests passed and unfortunately also masking multiple actual test failures.

Fixes #2139

@sentrivana sentrivana force-pushed the ivana/fix-2.7-tests branch from 0801e6c to cf89324 Compare June 5, 2023 10:04
@sentrivana sentrivana force-pushed the ivana/fix-2.7-tests branch from cf89324 to c279ef1 Compare June 5, 2023 10:04
@sentrivana sentrivana changed the title Fix UnicodeEncodeError on 2.7 Fix 2.7 tests Jun 5, 2023
@sentrivana
Copy link
Contributor Author

sentrivana commented Jun 6, 2023

Hey @Zylphrex, could you please take a look at the failing profiler tests for py 2.7 (here and here)?

TL;DR why this is coming up now: the CI wasn't actually executing the common/gevent test suites for py 2.7 and a handful of the tests were actually failing without us noticing.

I'll take care of the rest but don't know much about the profiler.

@Zylphrex
Copy link
Member

Zylphrex commented Jun 6, 2023

Hey @Zylphrex, could you please take a look at the failing profiler tests for py 2.7 (here and here)?

TL;DR why this is coming up now: the CI wasn't actually executing the common/gevent test suites for py 2.7 and a handful of the tests were actually failing without us noticing.

I'll take care of the rest but don't know much about the profiler.

You can disable the profiler tests for 2.7 as it's only meant to work with >3.

@sentrivana
Copy link
Contributor Author

Hey @Zylphrex, could you please take a look at the failing profiler tests for py 2.7 (here and here)?
TL;DR why this is coming up now: the CI wasn't actually executing the common/gevent test suites for py 2.7 and a handful of the tests were actually failing without us noticing.
I'll take care of the rest but don't know much about the profiler.

You can disable the profiler tests for 2.7 as it's only meant to work with >3.

Ok, will do, thanks!

@sentrivana sentrivana marked this pull request as ready for review June 6, 2023 15:25
Comment on lines +108 to +110
assert event["_meta"]["extra"]["auth"] == {"": {"rem": [["!config", "s"]]}}
assert event["_meta"]["breadcrumbs"] == {
"values": {"0": {"data": {"password": {"": {"rem": [["!config", "s"]]}}}}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed this because in py2.7 there's an additional 'sys.argv': {'': {'len': 11}}}} in event["_meta"]["extra"], I'm guessing from here. Unsure why this only happens on 2.7 but it doesn't seem to be something that needs to be fixed.

@@ -124,8 +122,8 @@ def test_span_data_scrubbing(sentry_init, capture_events):

(event,) = events
assert event["spans"][0]["data"] == {"password": "[Filtered]", "datafoo": "databar"}
assert event["_meta"] == {
"spans": {"0": {"data": {"password": {"": {"rem": [["!config", "s"]]}}}}}
assert event["_meta"]["spans"] == {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here ^

@sentrivana sentrivana requested a review from antonpirker June 7, 2023 07:51
Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

lgtm

@sentrivana sentrivana merged commit 55e5e39 into master Jun 7, 2023
@sentrivana sentrivana deleted the ivana/fix-2.7-tests branch June 7, 2023 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

py2.7 common CI tests green even on failure
3 participants