Skip to content

[python] add test for suspicious import errors#3866

Merged
christophe-papazian merged 7 commits intomainfrom
christophe-papazian/python_specific_test
Jan 21, 2025
Merged

[python] add test for suspicious import errors#3866
christophe-papazian merged 7 commits intomainfrom
christophe-papazian/python_specific_test

Conversation

@christophe-papazian
Copy link
Copy Markdown
Contributor

@christophe-papazian christophe-papazian commented Jan 21, 2025

Motivation

A suspicious import error was found in some system tests logs. This is related to a bad interaction between gevent and ctypes leading to an import failure when the appsec processor or the api manager is loaded.
This was fixed by DataDog/dd-trace-py#11987

As is seems to only be detectable through end to end test with a gunicorn based weblog, a system test was needed to add a regression test.

Changes

  • add a scenario for language specific tests
  • add a test file for python only tests, with a specific class test checking for import error through the circular import message on stderr
  • enable the new test for python on v2.20.0.dev. The test is currently failing on all gunicorn based weblog on prod, as expected.

APPSEC-56449

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes (if something not related to your task is failing, you can ignore it)
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner. We're working on refining the codeowners file quickly.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • If PR title starts with [<language>], double-check that only <language> is impacted by the change
  • No system-tests internal is modified. Otherwise, I have the approval from R&P team
  • CI is green, or failing jobs are not related to this change (and you are 100% sure about this statement)
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added (or removed)?

@christophe-papazian christophe-papazian marked this pull request as ready for review January 21, 2025 16:11
@christophe-papazian christophe-papazian requested review from a team as code owners January 21, 2025 16:11
@christophe-papazian christophe-papazian requested review from lievan and nsrip-dd and removed request for a team January 21, 2025 16:11
Copy link
Copy Markdown
Collaborator

@cbeauchesne cbeauchesne left a comment

Choose a reason for hiding this comment

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

Just two lines to remove, then all good

Comment thread tests/appsec/test_only_python.py Outdated
@christophe-papazian christophe-papazian merged commit f5db022 into main Jan 21, 2025
@christophe-papazian christophe-papazian deleted the christophe-papazian/python_specific_test branch January 21, 2025 17:18
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.

3 participants