Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Closes #10611 - Fix exit status of Flank #10612

Merged
merged 1 commit into from
May 14, 2020

Conversation

AaronMT
Copy link
Contributor

@AaronMT AaronMT commented May 12, 2020

Closes #10611

@AaronMT AaronMT added the eng:automation Build automation, Continuous integration, .. label May 12, 2020
@AaronMT AaronMT self-assigned this May 12, 2020
Copy link
Contributor

@rpappalax rpappalax left a comment

Choose a reason for hiding this comment

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

I'm not sure this will work. The reason why we use a function to set the exit code is that we still want some artifacts to be written regardless of pass or fail outcome. Most of the terminate on fail bash functions will end the script abruptly. I think we still need to capture the return code from the Java execution in a variable and hold onto it til the end. Not sure if pipefail is doing that

@AaronMT
Copy link
Contributor Author

AaronMT commented May 13, 2020

I'm not sure this will work. The reason why we use a function to set the exit code is that we still want some artifacts to be written regardless of pass or fail outcome. Most of the terminate on fail bash functions will end the script abruptly. I think we still need to capture the return code from the Java execution in a variable and hold onto it til the end. Not sure if pipefail is doing that

Setting pipefail will get us the the exit code of the process that's piped to another (sets the exit status $? to the exit code of the last program to exit non-zero (or zero if all exited successfully)).

E.g, what's happening now

$ false | true; echo $?
0

and after (see the ui-test-x86 debug failed above showing the right result now)

$ set -o pipefail
$ false | true; echo $?
1

@rpappalax
Copy link
Contributor

I'm not sure this will work. The reason why we use a function to set the exit code is that we still want some artifacts to be written regardless of pass or fail outcome. Most of the terminate on fail bash functions will end the script abruptly. I think we still need to capture the return code from the Java execution in a variable and hold onto it til the end. Not sure if pipefail is doing that

Setting pipefail will get us the the exit code of the process that's piped to another (sets the exit status $? to the exit code of the last program to exit non-zero (or zero if all exited successfully)).

E.g, what's happening now

$ false | true; echo $?
0

and after (see the ui-test-x86 debug failed above showing the right result now)

$ set -o pipefail
$ false | true; echo $?
1

I'm not sure this will work. The reason why we use a function to set the exit code is that we still want some artifacts to be written regardless of pass or fail outcome. Most of the terminate on fail bash functions will end the script abruptly. I think we still need to capture the return code from the Java execution in a variable and hold onto it til the end. Not sure if pipefail is doing that

Setting pipefail will get us the the exit code of the process that's piped to another (sets the exit status $? to the exit code of the last program to exit non-zero (or zero if all exited successfully)).

E.g, what's happening now

$ false | true; echo $?
0

and after (see the ui-test-x86 debug failed above showing the right result now)

$ set -o pipefail
$ false | true; echo $?
1

per our zoom sesion LGTM 👍

@codecov-io
Copy link

Codecov Report

Merging #10612 into master will increase coverage by 0.20%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #10612      +/-   ##
============================================
+ Coverage     19.20%   19.40%   +0.20%     
- Complexity      599      602       +3     
============================================
  Files           360      359       -1     
  Lines         14801    14638     -163     
  Branches       2001     1967      -34     
============================================
- Hits           2842     2841       -1     
+ Misses        11699    11537     -162     
  Partials        260      260              
Impacted Files Coverage Δ Complexity Δ
...g/mozilla/fenix/settings/logins/SavedLoginsView.kt 0.00% <0.00%> (-16.22%) 0.00% <0.00%> (ø%)
...ix/home/sessioncontrol/SessionControlController.kt 71.17% <0.00%> (-0.91%) 0.00% <0.00%> (ø%)
...pp/src/main/java/org/mozilla/fenix/FeatureFlags.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...rc/main/java/org/mozilla/fenix/FenixApplication.kt 12.24% <0.00%> (ø) 4.00% <0.00%> (ø%)
...g/mozilla/fenix/addons/AddonsManagementFragment.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...x/home/sessioncontrol/viewholders/TabViewHolder.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...me/sessioncontrol/viewholders/TopSiteViewHolder.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
.../sessioncontrol/viewholders/TabHeaderViewHolder.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...sessioncontrol/viewholders/CollectionViewHolder.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...oncontrol/viewholders/TabInCollectionViewHolder.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7993f10...34cd540. Read the comment docs.

@AaronMT AaronMT merged commit ea9d689 into mozilla-mobile:master May 14, 2020
@liuche liuche mentioned this pull request May 19, 2020
32 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng:automation Build automation, Continuous integration, ..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Fenix UI test reports logging success with failures
3 participants