Skip to content

Add tests for Service Extension (Envoy External Processing)#3377

Merged
e-n-0 merged 13 commits intomainfrom
flavien/external_proc_tests
Jan 16, 2025
Merged

Add tests for Service Extension (Envoy External Processing)#3377
e-n-0 merged 13 commits intomainfrom
flavien/external_proc_tests

Conversation

@e-n-0
Copy link
Copy Markdown
Member

@e-n-0 e-n-0 commented Nov 5, 2024

Motivation

Tests the new ASM Integration Service Extension.

Changes

Import tests from APM and ASM that applies to the Service Extension.
The import is using class generation to correctly import tests that should work and skip the ones that will not work.

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)?

@e-n-0 e-n-0 force-pushed the flavien/external_proc_tests branch 3 times, most recently from a64f21b to 1ff2cb4 Compare November 5, 2024 18:54
@e-n-0 e-n-0 marked this pull request as ready for review November 5, 2024 18:55
@e-n-0 e-n-0 requested a review from a team as a code owner November 5, 2024 18:55
Comment thread tests/external_processing/test_apm.py Outdated
@e-n-0
Copy link
Copy Markdown
Member Author

e-n-0 commented Dec 20, 2024

It's been some times since we discussed about this.
From what I remember, we divided our tasks:

  • @e-n-0: Duplicate all my tests classes to use base class and pass all the tests that I want to skip with an @irrelevent tag
  • @e-n-0: Use separate scenario (inside the external-processing group) for special case where a rule set is defined
  • @cbeauchesne: Make changes in the system-test core to allow the imports of the tests (tests currently failing because there have multiple scenarios linked to them due to the import)
  • @e-n-0 & @cbeauchesne: See how we could report the results to the Feature Parity Dashboard

Comment thread tests/external_processing/test_apm.py Outdated
Comment thread tests/external_processing/test_apm.py Outdated
Comment thread tests/external_processing/test_asm_blocking.py Outdated
Comment thread tests/external_processing/test_asm_blocking.py Outdated
Comment thread utils/_context/containers.py Outdated
@e-n-0 e-n-0 force-pushed the flavien/external_proc_tests branch from bd44002 to 1eb5853 Compare December 23, 2024 11:56
@e-n-0 e-n-0 requested a review from cbeauchesne December 27, 2024 18:12
@cbeauchesne cbeauchesne requested a review from a team as a code owner December 31, 2024 14:04
@cbeauchesne
Copy link
Copy Markdown
Collaborator

@e-n-0 I've fixed the common-test-code case.

Ping me if you need any help to move forward.

@e-n-0 e-n-0 force-pushed the flavien/external_proc_tests branch from d764377 to a8727a5 Compare January 14, 2025 16:47
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.

Two mall details, then all good

Comment thread .github/workflows/run-external-processing.yml Outdated
Comment thread utils/_context/_scenarios/external_processing.py
@e-n-0 e-n-0 requested a review from cbeauchesne January 15, 2025 16:11
@e-n-0 e-n-0 merged commit 962495b into main Jan 16, 2025
@e-n-0 e-n-0 deleted the flavien/external_proc_tests branch January 16, 2025 11:40
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.

2 participants