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

Change integration testing #3194

Merged
merged 3 commits into from
Aug 28, 2024

Conversation

jamshale
Copy link
Contributor

@jamshale jamshale commented Aug 22, 2024

Changes the automated integration testing so that there is 3 different types of tests.

  1. BDD Interop tests (AATH)- During a PR the workflow will clone AATH and then locate the forked repo location. Adjust the target for the tests to the forked repo and branch and run the acapy <-> acapy tests. These take about 43 mins. A little bit longer than the existing integration tests. These tests are more thorough and we want to encourage coverage here for the interop profile. The biggest problem here is that tests will need to be updated in AATH if they fail from aca-py, but I think that this shouldn't happen often as the interop protocols should be stable.
  2. Local BDD Tests - These are the current integration tests, but they have been scoped down considerably so they don't cover the same things as the AATH tests. They will still be available for those who like the Behave/Gherkin and demo agent style of tests. These only take about 6 minutes now and only include the endorsement tests and the anoncreds upgrade. We could still make it so all the tests run on releases. I want to make sure we aren't losing coverage removing all these tests, but I don't think we are.
  3. Scenario testing - These tests will use the acapy-controller (minimal-example) library created by Indicio. The examples have been copied from their repo as base tests. I thought about doing something trickier by cloning the repo but don't think it's necessary. If Indicio wants to copy the tests from acapy or vice versa then we can just copy and paste them. These tests only take about 3 minutes right now.

The scenario testing could probably be extended to have for e2e tests like what happens in the aries-acapy-tools repo. I imagine being able to test things like shut downs and upgrades and changing configurations from this setup.

TODO:

  • Adjust the AATH tags and run a smaller subset on PR's
  • What to do for nightly/release/manual workflow runs? Currently same as existing.
  • Tests are all pointing to http://test.bcovrin.vonx.io/ ledger and tails server. Maybe change to dev so we can wipe the env more often?

@jamshale jamshale changed the title Change testing [WIP] Change integration testing Aug 22, 2024
@jamshale jamshale marked this pull request as ready for review August 22, 2024 20:09
@jamshale jamshale linked an issue Aug 22, 2024 that may be closed by this pull request
@jamshale jamshale marked this pull request as draft August 26, 2024 19:40
@jamshale jamshale marked this pull request as ready for review August 26, 2024 21:26
@jamshale
Copy link
Contributor Author

jamshale commented Aug 26, 2024

The remaining warnings aren't an issue. Using http inside the test container network.

@jamshale jamshale force-pushed the change-testing branch 2 times, most recently from fad5fd7 to 298d418 Compare August 26, 2024 22:46
@jamshale jamshale changed the title [WIP] Change integration testing Change integration testing Aug 27, 2024
@jamshale jamshale requested review from ianco, dbluhm and esune August 27, 2024 19:25
Copy link
Member

@esune esune left a comment

Choose a reason for hiding this comment

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

Left a couple minor comments, but overall looks good to me.
Still wondering about executing ALL interop tests every time a PR gets opened/updated/reopened as it is potentially a big overhead and use of executor time. We could maybe create a matrix mapping code paths with test tags and dynamically specify which sets to execute (e.g.: only issue-credential and not connections) if nothing has changed, but this would definitely be for a future improvement 🤔

docs/testing/BDDTests.md Outdated Show resolved Hide resolved
docs/testing/IntegrationTests.md Outdated Show resolved Hide resolved
scenarios/Dockerfile Show resolved Hide resolved
scenarios/pyproject.toml Show resolved Hide resolved
.github/workflows/bdd-integration-tests.yml Show resolved Hide resolved
@jamshale
Copy link
Contributor Author

jamshale commented Aug 27, 2024

Left a couple minor comments, but overall looks good to me. Still wondering about executing ALL interop tests every time a PR gets opened/updated/reopened as it is potentially a big overhead and use of executor time. We could maybe create a matrix mapping code paths with test tags and dynamically specify which sets to execute (e.g.: only issue-credential and not connections) if nothing has changed, but this would definitely be for a future improvement 🤔

I do think that's a good idea and I could do it in an upcoming PR. I think we should only run a subset that only includes most of the happy path flows in AATH and the new scenarios. They will catch a vast majority of mistakes. Then for nightly and release runs we could do a more thorough testing suite on off hours.

The ultimate way would be to only run tests which matched the code changes. I don't think there's really a path forward for this with how complicated the codebase can be.

ianco
ianco previously approved these changes Aug 28, 2024
@jamshale
Copy link
Contributor Author

@esune I fixed the spelling mistakes and change the acapy-controller dependency in scenarios to target a release.

I'm going to create 3 tickets for additional work.

  1. Scoping the PR tests from AATH down
  2. Only run integration tests when there's source code or testing changes.
  3. Rework the nightly and release testing with scenarios and AATH testing.

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
15 Security Hotspots
0.0% Coverage on New Code (required ≥ 80%)
9.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@esune esune self-requested a review August 28, 2024 22:42
@jamshale jamshale merged commit fcbfb2f into openwallet-foundation:main Aug 28, 2024
9 of 10 checks passed
@jamshale jamshale deleted the change-testing branch August 29, 2024 16:25
ff137 pushed a commit to ff137/acapy that referenced this pull request Aug 30, 2024
* Change integration testing plan

Signed-off-by: jamshale <[email protected]>

* Fix grammer and spelling

Signed-off-by: jamshale <[email protected]>

* Use tag release for acapy-controller dependency

Signed-off-by: jamshale <[email protected]>

---------

Signed-off-by: jamshale <[email protected]>
darshilnb pushed a commit to Northern-Block/aries-cloudagent-python that referenced this pull request Sep 5, 2024
* Change integration testing plan

Signed-off-by: jamshale <[email protected]>

* Fix grammer and spelling

Signed-off-by: jamshale <[email protected]>

* Use tag release for acapy-controller dependency

Signed-off-by: jamshale <[email protected]>

---------

Signed-off-by: jamshale <[email protected]>
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.

Formalize a test plan
3 participants