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

gha: add neoeden job to test.yml workflow #1045

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

uncleDecart
Copy link
Member

Instead of adding additional workflow to eden, we run neoeden alongside with older version intent is that end user (EVE CI/CD pipeline) will not require any changes, it will still call test.yml there will be an additional test suite. End goal is to gradually move all the test suites to golang-based tests, so at some point smoke tests will be running in neoeden, or networking tests, etc. etc.

@europaul
Copy link
Contributor

europaul commented Dec 2, 2024

@uncleDecart but do we really need this workflow? I thought that golang tests are just compiled and run with the rest of Eden tests in the regular test workflow

Comment on lines 64 to 66
run: cd tests/sec && go test
shell: bash
working-directory: "./eden"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
run: cd tests/sec && go test
shell: bash
working-directory: "./eden"
run: go test
shell: bash
working-directory: "./eden/tests/sec"

will it work like this? 🤔 then you don't need to change the directory back

Instead of adding additional workflow to eden, we run neoeden alongside
with older version intent is that end user (EVE CI/CD pipeline) will not
require any changes, it will still call test.yml there will be an
additional test suite. End goal is to gradually move all the test suites
to golang-based tests, so at some point smoke tests will be running in
neoeden, or networking tests, etc. etc.

Signed-off-by: Pavel Abramov <[email protected]>
artifact_run_id: ${{ inputs.artifact_run_id }}
require_virtualization: ${{ inputs.require_virtualization }}
- name: Run security tests
run: cd tests/sec && go test
Copy link
Contributor

Choose a reason for hiding this comment

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

kudos to @shjala for the following comment:
why don't we put all commands calling the tests in this workflow in a bash script and call that script from here

Advantages:

  • you don't need to change GHA every time you add a new test - you add it to that workflow bash script instead
  • your newly added tests run immediately on the PR you added them in, because there is no change to the GHA itself

Disadvantages:

  • some security concerns about the latter advantage and how people might change the bash script on their PR

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