Skip to content

Run all embedding tests & fix appdata tests#433

Merged
klmcadams merged 11 commits into
mainfrom
fix/private-appdata-tests
Oct 3, 2023
Merged

Run all embedding tests & fix appdata tests#433
klmcadams merged 11 commits into
mainfrom
fix/private-appdata-tests

Conversation

@klmcadams

Copy link
Copy Markdown
Contributor
  • Fixed normal_appdata and private_appdata tests
  • Allow all embedding tests to run in workflow

@github-actions github-actions Bot added maintenance Package and maintenance related CI/CD bug Something isn't working labels Oct 3, 2023
@klmcadams klmcadams changed the title Fixed appdata tests Run all embedding tests & fix appdata tests Oct 3, 2023
Comment thread tests/embedding/test_app.py Outdated
@klmcadams klmcadams requested review from RobPasMue and koubaa October 3, 2023 17:32

@koubaa koubaa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

approving.. but I'm not sure I understand what was wrong with the appdata tests

@klmcadams

Copy link
Copy Markdown
Contributor Author

approving.. but I'm not sure I understand what was wrong with the appdata tests

Whenever we would run the appdata tests in the container, they would fail because of the crash on shutdown in Linux, so it was returning a signal abort and failing them (I had previously used subprocess.check_call). I adjusted them to be similar to the tests/embedding/test_logger.py tests, where they look for a specific string in the stdout to make sure it's working correctly instead

@koubaa

koubaa commented Oct 3, 2023

Copy link
Copy Markdown
Contributor

@klmcadams oh! makes sense, thanks!

@klmcadams

Copy link
Copy Markdown
Contributor Author

@klmcadams oh! makes sense, thanks!

No problem!

@klmcadams klmcadams merged commit b6500b6 into main Oct 3, 2023
@klmcadams klmcadams deleted the fix/private-appdata-tests branch October 3, 2023 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working maintenance Package and maintenance related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants