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

chore: Adding job step to conditionally execute e2e tests #185

Closed
wants to merge 2 commits into from

Conversation

priteshbandi
Copy link
Contributor

@priteshbandi priteshbandi commented Jan 20, 2024

chore: Adding job step to conditionally execute e2e tests.

This is required for notaryproject/notation-plugin-framework-go#13

@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2bc927b) 89.32% compared to head (1931696) 89.32%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #185   +/-   ##
=======================================
  Coverage   89.32%   89.32%           
=======================================
  Files          21       21           
  Lines        1724     1724           
=======================================
  Hits         1540     1540           
  Misses        149      149           
  Partials       35       35           

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

@JeyJeyGao
Copy link
Contributor

@priteshbandi Is this for notation-plugin-framework-go?
For notation-core-go, it doesn't need an E2E test. I'm not sure if it is appropriate to place the workflow here.

@priteshbandi
Copy link
Contributor Author

@priteshbandi Is this for notation-plugin-framework-go? For notation-core-go, it doesn't need an E2E test. I'm not sure if it is appropriate to place the workflow here.

Yes its for notaryproject/notation-plugin-framework-go#13. I am thinking of expanding reusable workflow to conditionally support support e2e tests.

@JeyJeyGao
Copy link
Contributor

notation-plugin-framework-go: .github/workflows/build.yml

Can we create a new e2e job and depend on build. For example:

jobs:
  build:
    uses: notaryproject/notation-core-go/.github/workflows/reusable-build.yml@main

  e2e:
    needs: build
    runs-on: ubuntu-latest
    steps:
      - name: Checkout code
        uses: actions/checkout@v2
      # Add the steps for your e2e test here.
      - name: Run E2E Tests
        run: # Your command to run E2E tests

Then, we don't need to update the reusable-build in this repo. @priteshbandi

@priteshbandi
Copy link
Contributor Author

Yes, that works but why do we dont want to add e2e tests in reusable-builds ?

@JeyJeyGao
Copy link
Contributor

@priteshbandi I think adding an E2E workflow here will work, but it's not elegant because it will be hard to understand why we need it if one doesn't know that the E2E is for another repository. Are there another other repos also use reusable-builds and need e2e? Or it just be used in notation-plugin-framework-go?

@priteshbandi
Copy link
Contributor Author

priteshbandi commented Jan 23, 2024

@priteshbandi I think adding an E2E workflow here will work, but it's not elegant because it will be hard to understand why we need it if one doesn't know that the E2E is for another repository. Are there another other repos also use reusable-builds and need e2e? Or it just be used in notation-plugin-framework-go?

Yes, I agree its not elegant but the name file reusable-build.yml implies that its been used at multiple places (potentially outside repo). Also, I think we might use e2e (but not sure) tests for tspclient-go and notation-hashicorp-vault repos.

Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

run-e2e-tests is out of the scope of notation-core-go. Please propose the change in the corresponding repository.

@shizhMSFT
Copy link
Contributor

tspclient-go does not require E2E tests. Unit tests are sufficient.

notation CLI has its own build.yml file (see https://github.com/notaryproject/notation/blob/main/.github/workflows/build.yml)

@shizhMSFT
Copy link
Contributor

reusable-build.yml is intended to be reused by notation-core-go and notation-go. If this file does not fit your requirement, this file should not be re-used. A new workflow should be created instead.

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.

4 participants