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

Don't run slow/expensive CI jobs if faster jobs fail #361

Merged
merged 16 commits into from
Aug 31, 2023

Conversation

glhewett
Copy link
Contributor

@glhewett glhewett commented Aug 29, 2023

I am looking for a faster CI, and these were some obvious updates to make. Actually, I could not stop making changes. Here is a list of the changes in this PR.

  1. I have created dependencies between the jobs. This required consolidating the jobs into one workflow file. If the clang-format fails, the rest of the job will not run. Next, I have added the interop job as a gate to do the platform jobs. This job will not use sanitizers, but it should be a quick test of openssl1, openssl2, and interop. Once those two jobs succeed, then we will move on to the full platform tests with sanitizer and tidy.
  2. I have updated most of the actions to the latest, greatest.
  3. I have moved the vcpgk caching to cache the binaries instead of the installed dependencies. This is a feature of vcpkg and should be very efficient. Since we do not have vcpkg locked as a submodule, then we might have to rebuild some dependencies an update cache from time to time, but vcpkg does not take a long time. Also, by consolidating the jobs, we have reduced a bit of the vcpkg repetition.
  4. We will not run the longer running jobs if it in draft status (not ready_for_review)
Screenshot 2023-08-30 at 11 39 34 AM

Copy link
Contributor

@bifurcation bifurcation left a comment

Choose a reason for hiding this comment

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

Nit: The names of the CI tasks are inconsistent. I would suggest:

  • Capitalize the first word, lower case thereafter
  • Use "OpenSSL 1.1" and "OpenSSL 3" consistently
jobs:
  formatting-check:
    name: Formatting check

  quick-linux-interop-check:
    needs: formatting-check
    name: Quick Linux Check and Interop
    runs-on: ubuntu-latest
    - name: Dependencies (ubuntu)
    - name: Restore cache
    - name: Configure (OpenSSL 1.1)
    - name: Build (OpenSSL 1.1)
    - name: Test (OpenSSL 1.1)
    - name: Build (Interop Harness)
    - name: Test self-interop
    - name: Test interop on test vectors
    - name: Test gRPC live interop with self
    - name: Configure (OpenSSL 3)
    - name: Build (OpenSSL 3)
    - name: Unit tests (OpenSSL 3)

  platform-sanitizer-tests:
    needs: quick-linux-interop-check
    name: Build and test platforms using sanitizers and clang-tidy

    steps:
    - name: Dependencies (macos)
    - name: Dependencies (ubuntu)
    - name: Restore cache
    - name: Configure (OpenSSL 1.1)
    - name: Build (OpenSSL 1.1)
    - name: Test (OpenSSL 1.1)
    - name: Configure (OpenSSL 3)
    - name: Build (OpenSSL 3)
    - name: Test (OpenSSL 3)

@bifurcation
Copy link
Contributor

Also it seems like we have Configure/Build/Test everywhere except for the interop harness, which only has Build/Test. Let's consolidate the Configure/Build steps into just Build.

@bifurcation bifurcation changed the title Bold updates to CI Don't run slow/expensive CI jobs if faster jobs fail Aug 30, 2023
@glhewett
Copy link
Contributor Author

Great feedback. I had not taken the higher level view yet. I have responded to all of you feedback.

@glhewett glhewett marked this pull request as draft August 30, 2023 16:53
@glhewett glhewett marked this pull request as ready for review August 30, 2023 18:04
@bifurcation bifurcation merged commit 7bfa991 into main Aug 31, 2023
11 checks passed
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