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

Fix integration tests #114

Merged
merged 3 commits into from
Oct 14, 2021
Merged

Conversation

ruuda
Copy link
Contributor

@ruuda ruuda commented Oct 7, 2021

I was unable to run the integration tests, for a few reasons:

  • vault secrets disable fails with

     Error disabling secrets engine at secret/: Delete "http://localhost:8200/v1/sys/mounts/secret": dial tcp: lookup localhost: no such host
    

    When I change localhost to 127.0.0.1 this error goes away.

  • The VAULT_* variables set in integration_test.sh do not propagate into Vaultenv when Stack uses Nix; it clears the environment, unless we pass --no-nix-pure.

  • Python 3.7 was hard-coded. Add Python to the Nix development environment instead.

  • In the Python integration test, subprocess.run with env set lost the original PATH, and when vault is taken from the Nix store, we need to preserve that PATH.

With these changes, I can run the integration tests, and they all pass.

Without this, Nix clears the environment, and then the VAULT_* variables
that are set by integration_test.sh do not get propagated properly.

Also, use the localhost ip instead of "localhost", because for some
reason, "localhost" fails to resolve on my machine.
@ruuda
Copy link
Contributor Author

ruuda commented Oct 7, 2021

Would be good to also run these on CI, but this is difficult for me to add because Semaphore doesn’t seem to run on pull requests that I open.

@maartenberg
Copy link
Member

Semaphore doesn’t seem to run on pull requests that I open

Hi! I've enabled "approval mode" for Semaphore now, so the CI should run if we comment the following on a PR:

/sem-approve

@maartenberg
Copy link
Member

Reopening this PR to make it actually work.

@maartenberg maartenberg reopened this Oct 14, 2021
@maartenberg
Copy link
Member

/sem-approve

Copy link
Member

@maartenberg maartenberg left a comment

Choose a reason for hiding this comment

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

LGTM, the tests also work on CI.
Thanks for fixing this!

@maartenberg
Copy link
Member

@OpsBotPrime merge

@maartenberg
Copy link
Member

@OpsBotPrime driemaal is scheepsrecht

@OpsBotPrime
Copy link
Contributor

Pull request approved for merge by @maartenberg, rebasing now.

@OpsBotPrime
Copy link
Contributor

Rebased as fb2de1f, waiting for CI …

@OpsBotPrime OpsBotPrime merged commit fb2de1f into channable:master Oct 14, 2021
@ruuda ruuda deleted the fix-integration-tests branch October 15, 2021 10:30
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.

3 participants