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

ci: Make the mobile-cc CI job run on all Envoy changes #33162

Merged
merged 2 commits into from
Mar 28, 2024

Conversation

abeyad
Copy link
Contributor

@abeyad abeyad commented Mar 27, 2024

It's important for us to run the mobile C++ unit and integration tests on any Envoy change. If we don't, we run the risk of breaking Envoy Mobile with seemingly benign changes.

For example, #32775 caused Envoy Mobile to permafail because an Envoy Mobile unit test wasn't run (see #33158 for the fix).

This change ensures that the Envoy Mobile C++ tests run on all core Envoy changes. There aren't too many of these tests, so it shouldn't add much to the runtime of CI per PR.

It's important for us to run the mobile C++ unit and integration tests
on any Envoy change. If we don't, we run the risk of breaking Envoy
Mobile with seemingly benign changes.

For example, envoyproxy#32775 caused Envoy
Mobile to permafail because an Envoy Mobile unit test wasn't run (see
envoyproxy#33158 for the fix).

This change ensures that the Envoy Mobile C++ tests run on all core
Envoy changes. There aren't too many of these tests, so it shouldn't add
much to the runtime of CI per PR.

Signed-off-by: Ali Beyad <abeyad@google.com>
- mobile/.bazelrc
- mobile/**/*
- tools/code_format/check_format.py
- "**/*"
Copy link
Member

Choose a reason for hiding this comment

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

as discussed offline can we make this a bit more specific - something like

    - .bazelrc
    - .bazelversion
    - .github/config.yml
    - bazel/external/quiche.BUILD
    - bazel/repository_locations.bzl
    - mobile/**/*
    - source/**/*
    - test/?/*
    - envoy/**/*
    - api/**/*

probably we should update the others that run on everything similarly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the suggestion, made the change, let's see how CI does with it

Signed-off-by: Ali Beyad <abeyad@google.com>
Copy link
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @abeyad

@phlax phlax merged commit 4b096c3 into envoyproxy:main Mar 28, 2024
52 checks passed
alyssawilk pushed a commit to alyssawilk/envoy that referenced this pull request Apr 29, 2024
)

It's important for us to run the mobile C++ unit and integration tests
on any Envoy change. If we don't, we run the risk of breaking Envoy
Mobile with seemingly benign changes.

For example, envoyproxy#32775 caused Envoy
Mobile to permafail because an Envoy Mobile unit test wasn't run (see
envoyproxy#33158 for the fix).

This change ensures that the Envoy Mobile C++ tests run on all core
Envoy changes. There aren't too many of these tests, so it shouldn't add
much to the runtime of CI per PR.

Signed-off-by: Ali Beyad <abeyad@google.com>
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