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 option to prevent uploading results for Smart Flank #537

Merged
merged 6 commits into from
Apr 5, 2019

Conversation

elihart
Copy link
Contributor

@elihart elihart commented Apr 4, 2019

Hi, thanks for this great library! I noticed that https://github.com/TestArmada/flank/blob/master/docs/smart_flank.md mentioned the possible improvement to disable uploading smart flank results, so I implemented it.

I have recently added Flank to our test suite at Airbnb, and I'd love this support to allow us to run a subset of tests on some branches without overwriting the master set of results.

@CLAassistant
Copy link

CLAassistant commented Apr 4, 2019

CLA assistant check
All committers have signed the CLA.

@elihart elihart force-pushed the eli-optional_upload branch from a7ed2cf to 6f1819d Compare April 4, 2019 20:33
@bootstraponline
Copy link
Contributor

Thanks! That's definitely a feature we want to support.

@bootstraponline
Copy link
Contributor

If you run ./gradlew check locally it should auto fix some of the lint issues.

@codecov-io
Copy link

codecov-io commented Apr 4, 2019

Codecov Report

Merging #537 into master will not change coverage.
The diff coverage is 90%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #537   +/-   ##
=========================================
  Coverage     79.65%   79.65%           
  Complexity      563      563           
=========================================
  Files            77       77           
  Lines          2109     2109           
  Branches        276      276           
=========================================
  Hits           1680     1680           
  Misses          265      265           
  Partials        164      164

@elihart
Copy link
Contributor Author

elihart commented Apr 4, 2019

Thanks for the tip, fixed!

I was going off the contributing doc which says to use ./gradlew ktlintFormat, which threw me off. Would be nice to update that.

@bootstraponline
Copy link
Contributor

ah, yeah we replaced ktlint with detekt. I'll update the doc

## This can be disabled to prevent new results from overriding previous results.
## Default: true
# smart-flank-upload-enabled: true

## Disables sharding. Useful for parameterized tests.
# disable-sharding: false

Copy link
Contributor

Choose a reason for hiding this comment

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

I think for consistency with disable-sharding, I'd like the flag renamed to:

smart-flank-disable-upload

then in the code we can check args.smartFlankDisableUpload instead of !args.smartFlankUploadEnabled.

The description can be written like this:

Disables smart flank JUnit XML uploading. Useful for preventing timing data from being updated.

I'm happy to make both changes if you'd like.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure thing, I'll do it

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect, thanks. After that I'll merge the PR.

@elihart elihart force-pushed the eli-optional_upload branch from 7cfbc94 to aa5b368 Compare April 5, 2019 14:17
@elihart
Copy link
Contributor Author

elihart commented Apr 5, 2019

All cleaned up, I think it should be good to go. Thanks for the quick review!

Any idea when you might be able to push a new release with this change? I'd love to get it into our app soon.

@bootstraponline
Copy link
Contributor

Awesome work! The Flank release process is fully automated. The snapshot jar is released after every merged PR. The stable releases are auto uploaded after I tag a version in git. I'll tag a 5.1.0 release after merging.

@bootstraponline bootstraponline merged commit d119b44 into Flank:master Apr 5, 2019
@bootstraponline
Copy link
Contributor

image

@elihart elihart deleted the eli-optional_upload branch April 5, 2019 16:12
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.

4 participants