Skip to content

test: Add Testcontainers and Gherkin execution for our test-harness #101

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Nov 18, 2024
Merged

test: Add Testcontainers and Gherkin execution for our test-harness #101

merged 6 commits into from
Nov 18, 2024

Conversation

aepfli
Copy link
Member

@aepfli aepfli commented Nov 9, 2024 •
edited
Loading

This pull request adds a gherkin runner with test containers

This approach:

  • uses Testcontainers to spin up the docker images
  • runs the test suites dynamically based on the spec and test-harness

Issues detected during this implementation

@aepfli aepfli force-pushed the feat/gherkin_and_testcontainers branch from 47f827d to ff96b1f Compare November 9, 2024 19:56
@codecov Codecov
Copy link

codecov bot commented Nov 9, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.52%. Comparing base (d8e10c7) to head (dcbef10).
Report is 42 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #101      +/-   ##
==========================================
+ Coverage   90.55%   93.52%   +2.96%     
==========================================
  Files           8       12       +4     
  Lines         180      463     +283     
==========================================
+ Hits          163      433     +270     
- Misses         17       30      +13
Flag Coverage Δ
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

@aepfli aepfli changed the title Feat/gherkin and testcontainers feat: Add Testcontainers and Gherkin execution for our test-harness Nov 9, 2024
@aepfli aepfli force-pushed the feat/gherkin_and_testcontainers branch from ff96b1f to 6749d83 Compare November 9, 2024 19:58
@aepfli
Copy link
Member Author

aepfli commented Nov 9, 2024

None of the evaluations are working because I get a GRPC.unavailable error when I try to resolve them. This might be related to the open issues. @toddbaert @beeme1mr fyi

@aepfli
feat: Add Testcontainers and Gherkin execution for our test-harness
7fbc770 
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
@aepfli aepfli force-pushed the feat/gherkin_and_testcontainers branch from 6749d83 to 7fbc770 Compare November 9, 2024 20:00
@beeme1mr
Copy link
Member

@aepfli what issue are you referring to?

@aepfli
Copy link
Member Author

aepfli commented Nov 10, 2024

@aepfli what issue are you referring to?

nvm I found the issue, the docker container took longer than expected to be available - now there are only 4 test cases missing, and we run the basic test suite as we do for js for the rpc provider - reconnect is something which i will do at a later stage

@aepfli aepfli force-pushed the feat/gherkin_and_testcontainers branch 4 times, most recently from 3408e94 to 78ef2a6 Compare November 10, 2024 21:16
beeme1mr
beeme1mr approved these changes Nov 10, 2024
View reviewed changes
Copy link
Member

@beeme1mr beeme1mr left a comment

Choose a reason for hiding this comment

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

Could you please add a section in the contributing doc that explains how to run the e2e tests?

@aepfli
Copy link
Member Author

aepfli commented Nov 11, 2024

Could you please add a section in the contributing doc that explains how to run the e2e tests?

So far, the command has not changed for execution. It runs with hatch run test, like before - if we want to split this, I would extract this into something separate. My Python tooling knowledge is not in a state, where I can easily do this

@aepfli aepfli force-pushed the feat/gherkin_and_testcontainers branch 5 times, most recently from ee32cd7 to feca070 Compare November 11, 2024 12:42
@aepfli aepfli mentioned this pull request Nov 14, 2024
Closed
@aepfli
fixup: make tests run, only 4 tests are missing now
7c809e8 
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
@aepfli aepfli force-pushed the feat/gherkin_and_testcontainers branch from feca070 to 7c809e8 Compare November 14, 2024 07:34
@aepfli aepfli marked this pull request as ready for review November 17, 2024 10:41
@aepfli aepfli requested a review from a team as a code owner November 17, 2024 10:41
@aepfli aepfli force-pushed the feat/gherkin_and_testcontainers branch 3 times, most recently from d1b47e6 to 3da62a3 Compare November 17, 2024 21:05
@aepfli aepfli force-pushed the feat/gherkin_and_testcontainers branch from 3da62a3 to 4d4f753 Compare November 17, 2024 21:13
@aepfli
Copy link
Member Author

aepfli commented Nov 17, 2024

Ready to be reviewed @toddbaert @beeme1mr @open-feature/sdk-python-approvers @open-feature/sdk-python-maintainers

@aepfli
fixup: adding gherkin tests for in-process via file
90526fe 
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
@aepfli aepfli force-pushed the feat/gherkin_and_testcontainers branch from 4d4f753 to 90526fe Compare November 17, 2024 21:26
@aepfli
fixup: deactivating the tests
6b012ba 
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
@toddbaert toddbaert changed the title feat: Add Testcontainers and Gherkin execution for our test-harness test: Add Testcontainers and Gherkin execution for our test-harness Nov 18, 2024
@toddbaert toddbaert self-requested a review November 18, 2024 14:24
toddbaert
toddbaert approved these changes Nov 18, 2024
View reviewed changes
Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

flagD -> flagd but otherwise approved, and very grateful! 🙏

@aepfli
fixup: renaming FlagD to Flagd
dcbef10 
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
@aepfli
Copy link
Member Author

aepfli commented Nov 18, 2024

flagD -> flagd but otherwise approved, and very grateful! 🙏

renamed - but we always pronounce it "flag D" else it looks more like "flagged" - just my few cents

@toddbaert
Copy link
Member

flagD -> flagd but otherwise approved, and very grateful! 🙏

renamed - but we always pronounce it "flag D" else it looks more like "flagged" - just my few cents

https://flagd.dev/reference/naming/

@aepfli aepfli mentioned this pull request Nov 18, 2024
Merged
@toddbaert toddbaert merged commit d936a47 into open-feature:main Nov 18, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@beeme1mr beeme1mr beeme1mr approved these changes

@toddbaert toddbaert toddbaert 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.
3 participants
@aepfli @beeme1mr @toddbaert