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

Add integration tests for Azure Remote #875

Merged
merged 10 commits into from Jul 11, 2018

Conversation

Projects
None yet
2 participants
@c-w
Copy link
Contributor

commented Jul 10, 2018

This is a follow-up to #868.

The main change of this pull request is to set up the Azure Storage emulator Azurite on the Travis CI machine via Docker and runs the Azure integration tests against the emulator (fcd0e34). An example of a successful build with this configuration on Travis can be found here.

The pull request also contains a number of related functional fixes:

  • Make the Azure remote compatible with Python 2 (8a32292).
  • Switch the exists implementation of the Azure remote to the same list-then-check logic as S3 (a438f77).
  • Make the Azure environment variables consistent between test and prod (581132b).
  • Add the Azure remote to an additional test suite (fcbc6d1).

Additionally, the pull request also includes some related refactoring:

  • Fix lint warnings in the data cloud tests file (1b1ca68).
  • Replace os.system and make should-run cloud test checks more robust (9724bd9).
  • Mark non-executed cloud tests explicitly as skipped (97dfc88).
  • Increase test runner verbosity (7897dd8).

c-w added some commits Jul 10, 2018

Replace os.system with subprocess.check_output
It is generally recommended to use the subprocess module instead of the
os module to run subshell commands given since the former makes it
easier to comunicate with the process, has an improved security model,
etc. More information is available in [1].

Note that this change uses `check_output` instead of `check_call` to
prevent the command output from being printed to stdout.

This change also introduces two additional enhancements:
- Skip the hadoop test if the hadoop binary is not available.
- Skip the ssh test if the ssh binary is not available.

[1] https://docs.python.org/3/library/subprocess.html#replacing-os-system
Enable Azure integration test on Travis CI
Note: the tests are run against the Azurite [1] local emulator for Azure
Blob Storage and not against a live service.

[1] https://github.com/azure/azurite
Make tests more verbose
This helps verify in the CI that the environment variables for the
integration tests are set up correctly since we can see exactly which
tests are being executed and which ones are being skipped.
@efiop

This comment has been minimized.

Copy link
Member

commented Jul 11, 2018

Hi @c-w !

Thank you for the follow up! Looks great! I see that our appveyor test is failing on Azure test, is there any chance you could also setup azurite for it?

Thanks,
Ruslan

@c-w

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2018

Working on it. Might be pushing a bunch of odd updates to the PR to re-trigger the build while I figure this out. Will ping you when this is ready to be reviewed again.

@c-w

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2018

Hi @efiop. Happy to report that the AppVeyor build now runs the Azure remote integration tests via Azurite. I tried three approaches to set this up:

  1. At first, I tried to run Azurite in Docker just like we do on Travis CI (via image: Visual Studio 2017), but by default the Docker install is set up to run Windows containers so the Azurite container wouldn't run. Switching to Linux containers (via docker-switch-linux) failed when spinning up the MobyLinuxVM, likely because the AppVeyor worker ran out of memory.
  2. I then tried to install and auto-start Azurite via Nuget (via Install-Package Azurite) but this didn't actually launch the process and the tests failed with a connection refused error.
  3. Installing Node (via Install-Product node) and the Azurite NPM package (via npm install -g azurite) and managing the Azurite process via Powershell (via Start-Process and Stop-Process) worked fine, so that's the solution implemented in 8f36587.
@efiop

This comment has been minimized.

Copy link
Member

commented Jul 11, 2018

Hi @c-w !

Thank you so much for the amazing work! Merged! I will release 0.11.0 with Azure support until tomorrow.

@efiop efiop merged commit 5f8b2f3 into iterative:master Jul 11, 2018

3 checks passed

codeclimate All good!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@c-w

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2018

Thanks for the merge! Looking forward to the package being released :)

@c-w c-w deleted the CatalystCode:azure-integration-tests branch Jul 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.