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: Print args when iOS tests are not found #1395

Merged
merged 6 commits into from
Dec 16, 2020

Conversation

adamfilipow92
Copy link
Contributor

@adamfilipow92 adamfilipow92 commented Dec 11, 2020

Fixes #1388

Test Plan

How do we know the code works?

  1. run ios test with config
gcloud:
  test: [path to zip]
  xctestrun-file: [path to xctestrun]
flank:
  test-targets:
    - nonExisting/Class
  1. Flank should print args before any validation (ios and android)
  2. Exit code should be 1
  3. Integration tests should works

Checklist

  • Integration tests works

@adamfilipow92 adamfilipow92 self-assigned this Dec 11, 2020
@adamfilipow92 adamfilipow92 marked this pull request as draft December 11, 2020 16:10
@github-actions
Copy link
Contributor

github-actions bot commented Dec 11, 2020

Timestamp: 2020-12-15 12:53:51
Buildscan url for ubuntu-workflow run 423204021
https://gradle.com/s/ormga2qul3che

@codecov-io
Copy link

codecov-io commented Dec 11, 2020

Codecov Report

Merging #1395 (a0a1bfe) into master (2529e16) will decrease coverage by 0.02%.
The diff coverage is 72.72%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1395      +/-   ##
============================================
- Coverage     77.31%   77.28%   -0.03%     
  Complexity      719      719              
============================================
  Files           250      250              
  Lines          4831     4830       -1     
  Branches        922      922              
============================================
- Hits           3735     3733       -2     
- Misses          595      596       +1     
  Partials        501      501              

@adamfilipow92 adamfilipow92 marked this pull request as ready for review December 11, 2020 18:55
@bootstraponline bootstraponline force-pushed the 1388-fix-output-inconsistency branch 4 times, most recently from 606e7ce to 0ccd96c Compare December 14, 2020 09:22
Copy link
Contributor

@jan-goral jan-goral left a comment

Choose a reason for hiding this comment

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

I don't like the validation is split between execution steps. Wouldn't be better to print args before any validation instead of breaking validation consistency?

WDYT? @pawelpasterz @piotradamczyk5 @adamfilipow92

@bootstraponline bootstraponline force-pushed the 1388-fix-output-inconsistency branch from 0ccd96c to a0d78ae Compare December 14, 2020 13:06
@adamfilipow92
Copy link
Contributor Author

I don't like the validation is split between execution steps. Wouldn't be better to print args before any validation instead of breaking validation consistency?

WDYT? @pawelpasterz @piotradamczyk5 @adamfilipow92

Agree

@adamfilipow92 adamfilipow92 marked this pull request as draft December 14, 2020 14:20
@bootstraponline bootstraponline changed the title fix: Print args when not found tests on iOS fix: Print args when tests are not found on iOS Dec 14, 2020
@bootstraponline bootstraponline changed the title fix: Print args when tests are not found on iOS fix: Print args when iOS tests are not found Dec 14, 2020
@pawelpasterz
Copy link
Contributor

I don't like the validation is split between execution steps. Wouldn't be better to print args before any validation instead of breaking validation consistency?

WDYT? @pawelpasterz @piotradamczyk5 @adamfilipow92

+1

I think printing args (always) is a good idea.

@adamfilipow92 adamfilipow92 marked this pull request as ready for review December 15, 2020 12:06
Copy link
Contributor

@Sloox Sloox left a comment

Choose a reason for hiding this comment

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

It appears it will fix #1389

@piotradamczyk5 piotradamczyk5 self-requested a review December 16, 2020 11:26
@adamfilipow92 adamfilipow92 merged commit 81bf670 into master Dec 16, 2020
@adamfilipow92 adamfilipow92 deleted the 1388-fix-output-inconsistency branch December 16, 2020 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Output inconsistency (ios vs android)
7 participants