Skip to content

Test/integrations #91

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

Merged
merged 13 commits into from
Jun 24, 2020
Merged

Test/integrations #91

merged 13 commits into from
Jun 24, 2020

Conversation

kiran-chaudhary
Copy link
Contributor

@kiran-chaudhary kiran-chaudhary commented Jun 2, 2020

Added setup for integration tests.

Copy link
Contributor

@antazoey antazoey left a comment

Choose a reason for hiding this comment

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

These are good!

@alanag13
Copy link
Contributor

alanag13 commented Jun 5, 2020

@kiran-chaudhary I was thinking about an approach this sort of like this:

  • create profile reading profile details from environment, if it does not already exist.
  • test execution.

Note that I am skipping the "delete all profiles" and "delete profile as part of cleanup activity" steps. This would allow us to set environment variables on our local machines for accounts that we already have profiles for to run these tests locally without having the profiles get deleted.
We could use this functionality in our CI as well -- the github actions are run on containers that get destroyed at the end of the run, so we would be creating the profile new every time.

Check out the bottom of this file:
https://github.com/code42/c42eventextractor/blob/master/.github/workflows/build.yml

and this one:
https://github.com/code42/c42eventextractor/blob/master/run_integrations.py

to see how I set this up for c42eventextractor.

One thing that occurs to me is that in order to make this automatable, we would need to add (optional) flags that allow us to skip confirmation prompts, and that allow for the password to be input as part of the create-profile command. Those should probably be their own tasks, I'll get stories created for them.

@kiran-chaudhary kiran-chaudhary force-pushed the test/integrations branch 2 times, most recently from c9978c3 to 4a75e96 Compare June 11, 2020 14:12
@kiran-chaudhary
Copy link
Contributor Author

Used pexpect module to pass password on prompt.

@alanag13
Copy link
Contributor

alanag13 commented Jun 22, 2020

@kiran-chaudhary FYI I will usually assume that PRs that are marked as draft are not considered ready by the author, (except immediately after opening and asking for initial feedback), so be sure to mark them as ready for review.

Just one small note about using os.remove instead of rm -f but this looks good. We would want commands to handle creating profiles but that can be done in a separate PR.

@kiran-chaudhary kiran-chaudhary marked this pull request as ready for review June 23, 2020 07:37
@kiran-chaudhary
Copy link
Contributor Author

@kiran-chaudhary FYI I will usually assume that PRs that are marked as draft are not considered ready by the author, (except immediately after opening and asking for initial feedback), so be sure to mark them as ready for review.

Just one small note about using os.remove instead of rm -f but this looks good. We would want commands to handle creating profiles but that can be done in a separate PR.

Sorry I missed changing back to ready for review.

@kiran-chaudhary kiran-chaudhary merged commit d7e1354 into master Jun 24, 2020
@kiran-chaudhary kiran-chaudhary deleted the test/integrations branch June 24, 2020 03: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.

3 participants