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

Integration tests with multiple configuration files #49

Merged
merged 11 commits into from
Mar 4, 2025

Conversation

wvengen
Copy link
Member

@wvengen wvengen commented Feb 26, 2025

An attempt.

  • Initial CI configuration
  • Test http auth #24
  • Implement -c argument for scrapyd-k8s
  • Check CI configuration functions
  • Check TEST_ environment variables are passed through in CI config (do they need export?)
  • Find a solution for the final kill %1 not working the very last time when running scrapyd-k8s directly (both docker and k8s; one thing to try would be to store the pid in a variable right after running it in the background, and killing the pid directly; or use another signal; may also be related to the note on termination in the next point) --> just needed to wait and ignore exit code
  • Find a solution for in-cluster scrapyd-k8s not coming up again soon enough in 2nd integration test (a likely cause is that scaling down scrapyd-k8s is taking a lot of time, perhaps we need to handle signals better so that it terminates earlier)
  • Make sure tests pass
  • Document how integration tests function, with config file adaptation, and how to run these tests easily there is no doc about this, fix later

still needs a change in scrapyd-k8s to support the `-c` argument to load
a specific configuration file, and allows multiple of them
(`configparser` supports a list argument when loading for multiple files).
@wvengen wvengen force-pushed the feature/integration-tests-multiple-config branch from 600060e to d527520 Compare February 26, 2025 19:47
@wvengen wvengen requested a review from vlerkin February 26, 2025 20:06
@wvengen
Copy link
Member Author

wvengen commented Feb 27, 2025

@vlerkin this is along the lines of what I thought. It keeps a better separation between the environment setup, and the actual test. And exactly the same test can be used for docker/k8s/in-cluster-k8s.

There are some small but important issues to resolve. Are you able to look into this? I think it needs a bit of Linux knowledge, let me know if you need pointers (or ask around).

@wvengen wvengen force-pushed the feature/integration-tests-multiple-config branch 4 times, most recently from 3692ec1 to 8b1fb44 Compare March 4, 2025 12:14
@wvengen
Copy link
Member Author

wvengen commented Mar 4, 2025

Ah, the k8s failure is actually very very simple: the service never comes up, because the health check is behind the auth. Bummer this issue is still open ... jpvanhal/flask-basicauth#11

@wvengen wvengen force-pushed the feature/integration-tests-multiple-config branch from 8b1fb44 to 001890c Compare March 4, 2025 13:16
@wvengen wvengen marked this pull request as ready for review March 4, 2025 13:20
@wvengen wvengen merged commit 3e48e84 into main Mar 4, 2025
5 checks passed
@wvengen wvengen deleted the feature/integration-tests-multiple-config branch March 4, 2025 13:23
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.

1 participant