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

add aws credentials to circleci system tests #2857

Merged
merged 4 commits into from
Sep 30, 2024

Conversation

wconti27
Copy link
Contributor

Description

add necessary auth for new aws system tests and add IDM scenarios of interest

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@wconti27 wconti27 requested a review from a team as a code owner September 16, 2024 19:30
@wconti27 wconti27 self-assigned this Sep 16, 2024
@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.49%. Comparing base (501f386) to head (1b68009).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2857   +/-   ##
=========================================
  Coverage     78.49%   78.49%           
  Complexity     2517     2517           
=========================================
  Files           173      173           
  Lines         18683    18683           
  Branches        975      975           
=========================================
  Hits          14665    14665           
  Misses         3481     3481           
  Partials        537      537           
Flag Coverage Δ
appsec-extension 69.12% <ø> (ø)
tracer-extension 78.18% <ø> (ø)
tracer-php 82.27% <ø> (ø)

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


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 501f386...1b68009. Read the comment docs.

@bwoebi
Copy link
Collaborator

bwoebi commented Sep 16, 2024

I sort-of don't really like having more secrets than needed in CI. What do we need aws credentials for, in system tests?
Can't these tests run locally, somehow? I'd strongly prefer having tests without external dependencies.

@wconti27
Copy link
Contributor Author

wconti27 commented Sep 17, 2024

I sort-of don't really like having more secrets than needed in CI. What do we need aws credentials for, in system tests? Can't these tests run locally, somehow? I'd strongly prefer having tests without external dependencies.

@bwoebi We need AWS tests for correctly testing propagation between Lambda and messaging systems (SQS, SNS, Kinesis). The previous 3rd party software that we were using (Localstack) was not cutting it. It was not a 1 to 1 matchup of feature support compared to actual AWS, and was providing both false negatives and false positives for our tests. This change has already been made in the other tracers.

Copy link
Collaborator

@bwoebi bwoebi left a comment

Choose a reason for hiding this comment

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

Okay, let's merge it.
But definitely first merge conti/fix-php-errors on the system-tests repo.

@wconti27 wconti27 merged commit f4b4015 into master Sep 30, 2024
692 of 697 checks passed
@wconti27 wconti27 deleted the conti/system-tests-ci-update branch September 30, 2024 19:20
@github-actions github-actions bot added this to the 1.4.0 milestone Sep 30, 2024
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