Making CI tests only on push

[vishnuchalla]

Type of change

• Refactor
• New feature
• Bug fix
• Optimization
• Documentation Update

Description

Making CI tests to run only on push because they use secrets that work only on the mainstream repo.

Checklist before requesting a review

• I have performed a self-review of my code.
• If it is a core feature, I have added thorough tests.

Testing

Will be tested against every PR after merge into main branch.

[shashank-boyapally]

/lgtm

[paigerube14]

shouldn't we try to run the tests on any given pr before its merged to know if its a breaking change?

[vishnuchalla]

shouldn't we try to run the tests on any given pr before its merged to know if its a breaking change?

Ideally yes. But the problem is github secrets are not available for fork PRs which will try to fetch our QE ES server credentials while executing the CI tests. Only main repo has access to secrets, which is a limitation with github. So the idea behind this change is to get tests triggered after a merge which will help us detect issues atleast before we cut a release solving our purpose.

[paigerube14]

@vishnuchalla thoughts on maybe just loading some data into the public es instance that we wouldn't have to use a secret but could still run pr based tests

[vishnuchalla]

@vishnuchalla thoughts on maybe just loading some data into the public es instance that we wouldn't have to use a secret but could still run pr based tests

Yeah, thought about it too. But the issue is we have to hardcode its URL in the tests which will make it available to the internet. So literally anyone will be able to use our ES instance and we will be charged for it.

[paigerube14]

Thoughts on adding in documentation somewhere on how to run the github tests locally. or even completely remove them as to run in github/actions and just have them documented to run before opening a pr

[jtaleric]

or even completely remove them as to run in github/actions and just have them documented to run before opening a pr

+1

I wonder instead of trying to use live data, we just store the json in a var to test? instead of querying ES, we just load the json, so we know the expected outcome?

@vishnuchalla / @shashank-boyapally - Could we close out on this?

[vishnuchalla]

or even completely remove them as to run in github/actions and just have them documented to run before opening a pr

+1

I wonder instead of trying to use live data, we just store the json in a var to test? instead of querying ES, we just load the json, so we know the expected outcome?

@vishnuchalla / @shashank-boyapally - Could we close out on this?

The idea was to have functional tests that are close to the ones that execute in our CI. So I think we should keep them as is but have clear documentation on how to trigger github actions for the ease of contributors.

Having the data input mocked as json will transition these tests into unit based rather than functionality taking us far away from our purpose.

[shashank-boyapally]

I think we can either add this in the readme on how to test before pushing the changes or have a contributing.md, either way works..

[jtaleric]

Having the data input mocked as json will transition these tests into unit based rather than functionality taking us far away from our purpose.

Orion shouldn't need to functionally test connectivity to ES / OpenSearch that is more of a fmatch function.

[vishnuchalla]

Having the data input mocked as json will transition these tests into unit based rather than functionality taking us far away from our purpose.

Orion shouldn't need to functionally test connectivity to ES / OpenSearch that is more of a fmatch function.

Its not about what orion has, these tests are all about what orion does in an end-to-end workflow.

[jtaleric]

Having the data input mocked as json will transition these tests into unit based rather than functionality taking us far away from our purpose.

Orion shouldn't need to functionally test connectivity to ES / OpenSearch that is more of a fmatch function.

Its not about what orion has, these tests are all about what orion does in an end-to-end workflow.

sure - the complexity is verifying the end to end -- however since we have limitations on connectivity for each PR either
A) We make the PR submitter do the testing because the CI lack functionality due to security.
B) Mock aspects JUST for the CI to ensure things being introduce do not break.

Having the CI run at push into main IMHO lacks why we have a CI - because if identifies a breaking change, we will have to revert vs testing IN the PR.

[vishnuchalla]

Updated with appropriate docs as discussed offline. PTAL 👋🏼

[shashank-boyapally]

Merging!