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

Upgrade testing for SQLite #432

Merged
merged 2 commits into from
Dec 6, 2022
Merged

Upgrade testing for SQLite #432

merged 2 commits into from
Dec 6, 2022

Conversation

YC
Copy link
Collaborator

@YC YC commented Dec 4, 2022

How it works:

  1. Download release version of Wakapi
  2. Run tests against release version
  3. Run current build of wakapi
  4. If the service can come up, it means that migrations succeeded

This PR also changes run_api_tests.sh to have two optional arguments database type and upgrade testing, which defaults to sqlite and no upgrade testing respectively.

Part of #407

@YC YC requested a review from muety December 4, 2022 07:08
@YC YC changed the title ci: upgrade testing for SQLite Upgrade testing for SQLite Dec 4, 2022
@github-actions
Copy link

github-actions bot commented Dec 4, 2022

Mayhem for API Automated API Testing Report

✔️ 🎆 0 Errors Found

⚠️ 1 Warnings Found

Rule Method Path Details
Timeout GET /compat/wakatime/v1/users/{user}/summaries ↗️

Testing details and issue reproduction found at https://mayhem4api.forallsecure.com/job/20321

@muety
Copy link
Owner

muety commented Dec 5, 2022

Great contribution! 🙏 A few remarks:

  • Can you please add a wait_for_wakapi statement before the first test run?
  • Could you add another 1-2 seconds sleep to the wait function? I noticed that even when the instance is up, the first one or two test cases still fail due to connection reset
  • If one of the tests fails, the Wakapi process is not properly terminated, but remains running and thus blocks the port
  • The second positional argument "y" looks a bit non-intuitive for an outside user. Could you make it --migrations or so?
  • Minor one: add logging before the release downloading
  • Minor one: lower-case variable names (for consistency)

@YC YC marked this pull request as draft December 6, 2022 07:38
@YC YC force-pushed the upgrade-testing branch from ff1370b to f5395e3 Compare December 6, 2022 07:40
@YC YC marked this pull request as ready for review December 6, 2022 07:40
@muety
Copy link
Owner

muety commented Dec 6, 2022

Awesome contribution, thanks you so much!

@muety muety merged commit db6dde3 into master Dec 6, 2022
@YC YC deleted the upgrade-testing branch December 16, 2022 23:36
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.

2 participants