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

GEODE-5443: Making the UITests a manually triggered pipeline. #2149

Merged
merged 3 commits into from Jul 30, 2018

Conversation

mhansonp
Copy link
Contributor

Co-authored-by: fsoutherland

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

  • Has your PR been rebased against the latest commit within the target branch (typically develop)?

  • Is your initial contribution a single, squashed commit?

  • Does gradlew build run cleanly?

  • Have you written or updated unit tests to verify your changes?

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

Note:

Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.

@mhansonp
Copy link
Contributor Author

@smgoller
Copy link
Contributor

After discussion with @monkeyherder, we came to the conclusion that having this job as part of the PR pipeline isn't useful, and this should be removed from that, and run as a calendar-triggered job that runs once a day or once a week and doesn't affect the pipeline status.

@upthewaterspout
Copy link
Contributor

Can't someone just run these tests manually themselves? If we are pulling these tests out because they are flaky and we don't think they have much value, we should just remove them from concourse entirely.

@mhansonp mhansonp force-pushed the GEODE-5443 branch 2 times, most recently from 9e7b139 to 6ee83d1 Compare July 18, 2018 00:09
@jake-at-work
Copy link
Contributor

@upthewaterspout not everyone has access to trigger job manually.

Until they are made stable they provide no useful feedback. At the very least they should come off the PR because we would be telling people to ignore red PR CI which is bad. If we keep them on the main CI at least the release manager can restart the job and make it green. Another option is to add to either or both pipelines the concept of "rerun failed tests".

@upthewaterspout
Copy link
Contributor

@pivotal-jbarrett - Hmm, maybe you misunderstood my comment. I'm saying the same thing that you are, we shouldn't be running these tests in any concourse jobs. If someone wants to run them, they can run them manually on their own computer.

@mhansonp
Copy link
Contributor Author

@upthewaterspout Your suggestion all but guarantees they are going to fall into disrepair. Even very rarely running them is better than nothing, the concession there, was to make a manually triggered CI. As it is far more public, I favor that approach. Also that way the build can easily be checked without tying up time running them locally for a one off check.

@jake-at-work
Copy link
Contributor

@upthewaterspout @mhansonp Ping!!! So this issue has gone unresolved for a week. I'm inclined to side with @upthewaterspout on this one and delete the task altogether until such time (if ever) it provides meaningful value to run them again.

@galen-pivotal
Copy link
Contributor

I'm with @upthewaterspout and @pivotal-jbarrett -- I think we should remove the task, especially for PR pipelines.

@mhansonp
Copy link
Contributor Author

Ok, I will work to get that done.

@mhansonp
Copy link
Contributor Author

mhansonp commented Jul 26, 2018

@upthewaterspout @pivotal-jbarrett @galen-pivotal Deleted the pipeline file, now working with Sean to verify that it gets deleted properly.

@mhansonp
Copy link
Contributor Author

Apparently this needs to merged before it will take affect in the PR pipeline.

@upthewaterspout upthewaterspout merged commit 82a5fe6 into apache:develop Jul 30, 2018
@mhansonp mhansonp deleted the GEODE-5443 branch April 21, 2020 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants