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

AATH ACA-Py Tests Are Failing -- Need Poetry Update #2459

Closed
swcurran opened this issue Sep 3, 2023 · 4 comments
Closed

AATH ACA-Py Tests Are Failing -- Need Poetry Update #2459

swcurran opened this issue Sep 3, 2023 · 4 comments
Assignees

Comments

@swcurran
Copy link
Contributor

swcurran commented Sep 3, 2023

The most recent run shows a lot of ACA-Py tests are now failing. I expected it would be something straight forward and have found one problem, but it did not fix all of the errors. Something in 0.10.1 has broken the tests.

The core issue is likely the update of ACA-Py to poetry #2436 -- confirmed that the commit before that worked, that commit fails. The AATH ACA-Py backchannel likely has to be updated to use poetry. I thought I could just make the same updates to the Dockerfiles, but I'm not exactly sure the best way to do that while retaining an important AATH feature. Notably, the ability to easily run AATH with the main branch, or any other branch/tag/commit. See comment below on what we do today.

Aside: What I did find that some tests are failing because the new (in 0.9.0...) option "--requests-through-public-did", seems to be needed. Oddly, I didn't see this failure coming up earlier. I've added a PR for it to AATH -- but that just fixes a single test, and I think that test has been failing for sometime.

To see a summary of the most recent test run: https://allure.vonx.io/api/allure-docker-service/projects/acapy-aip10/reports/latest/index.html?redirect=false#behaviors

To run all the tests:

To run just a single test, find the test you want to run, copy the tags for the test (e.g. @T001-RFC0025) and then paste them at the end of this command, with a -t in front of each tag (with or without the @).

  • `./manage run -d acapy-main -t -t ...

The run will startup the needed services and then run all of the tests in the acapy GHA test.

To run with a specific branch/tag/commit:

  • In your clone, update the file aries-backchannels/acapy/requirements-main.txt and edit the aca-py branch main to whatever you want -- e.g. a specific tag, branch or commit.
  • Repeat the runset (with acapy) or run command (with -d acapy-main)
@swcurran
Copy link
Contributor Author

swcurran commented Sep 4, 2023

@Gavinok, @esune -- looks like AATH ACA-Py Test Agent is failing after the Poetry upgrade. Oddly, I does seem to build, and run (and even pass some tests), but many tests fail. I'm guessing because some of the requirements are not being included in the build. All I know is that the commit before the poetry commit passed, after the poetry commit we get errors.

Let's talk about getting this addressed. Note the requirement above -- that when I clone/fork the AATH repo I can easily run with any version of ACA-Py (tag, branch, commit). Currently trivial to do -- see note in the first comment on this issue.

@swcurran swcurran changed the title AATH ACA-Py Tests Are Failing AATH ACA-Py Tests Are Failing -- Need Poetry Update Sep 4, 2023
@dbluhm
Copy link
Contributor

dbluhm commented Sep 5, 2023

Would it make sense to move AATH to using the nightly images instead of needing to manage Dockerfiles for ACA-Py in both this repo and the AATH repo?

@usingtechnology usingtechnology self-assigned this Sep 19, 2023
@usingtechnology
Copy link
Contributor

Posting here for a quick update... got a breakthrough on this. Need to test more locally and clean up the many experiments.
Upshot is that it is not really poetry, it appears that removing bin/aca-py in the poetry PR is the issue.

In pyproject.toml, this negates the need for that shell script in the built images:

[tool.poetry.scripts]
aca-py = "aries_cloudagent.__main__:script_main"

However, in AATH, it makes use of bin/aca-py and expects it is installed. No idea how the original call to aca-py works and the the subsequent ones don't, nor do I understand how that image is built (it must be docker cached) because on a new image build, it calls bin/aca-py to set a version.txt file. Technically the images should not even be built without bin/aca-py, but here we are.

Anyway, on to clean up and getting a PR in place for AATH.

@swcurran
Copy link
Contributor Author

swcurran commented Sep 24, 2023

Resolved in AATH. Issue was that in the poetry update the bin/aca-py executable was dropped as no longer needed, but AATH (and perhaps others) expected it to be present.

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

No branches or pull requests

3 participants