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

Remove test skipping on connection error #173

Merged
merged 3 commits into from
Jan 21, 2024

Conversation

philippgille
Copy link
Owner

@philippgille philippgille commented Jan 21, 2024

In the past, probably before we had the mage test all that automatically spins up Docker containers for the different store implementation tests, it was useful to do connection tests and skip tests when no connection could be established, to not fail an entire test run, but instead allow to check in the output log for any unexpected skips.

The Docker container start has been very reliable though, while OTOH I've been less often checking for skipped tests, and it's also cumbersome to check with the long test output (and not using tparse yet).

=> This PR removes the connection check and test skipping from almost all store implementation tests.

The exceptions:

  • tablestorage and tablestore: Their test containers don't yet work correctly. So far they need to be tested locally, with actual credentials to the production cloud services. We should revisit their documentation, because since I last checked they might have improved/fixed their Docker images or improved their documentation on how to make them work.
  • MySQL: For some reason the tests work fine locally (always) but never work in CI. Haven't found out why yet. As an improvement over the previous state we replace the connection test with a check for the test environment and only skip the test in CI, but not locally.

We leave the checkConnection functions in the test files even if they're not called, because they're very handy as soon as connection issues pop up, to find out if it's from the gokv client specific code or the connection in general.

@codecov-commenter
Copy link

codecov-commenter commented Jan 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6bcd482) 63.64% compared to head (7742dbb) 63.64%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #173   +/-   ##
=======================================
  Coverage   63.64%   63.64%           
=======================================
  Files          25       25           
  Lines        2107     2107           
=======================================
  Hits         1341     1341           
  Misses        651      651           
  Partials      115      115           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@philippgille philippgille marked this pull request as ready for review January 21, 2024 20:09
@philippgille philippgille merged commit 09cafca into master Jan 21, 2024
8 checks passed
@philippgille philippgille deleted the remove-test-skipping-on-connection-error branch January 21, 2024 20:09
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.

None yet

2 participants