From b3848ecdc8ade17a2acf80d2c52223221fcb71dc Mon Sep 17 00:00:00 2001 From: Jay Soffian Date: Tue, 23 Apr 2024 19:50:11 -0400 Subject: [PATCH] Fix trainer failure result handling with objective C tests When trainer is parsing an xcresult bundle which contains test failures, it has an `ActionTestMetadata` object that is identified one way (by "identifier") and it's trying to find the corresponding `TestFailureIssueSummary` object that's identified another way (by `testCaseName`). These values are not the same, so in order to locate the correct `TestFailureIssueSummary` object it needs to transform the `testCaseName` in the same way that Xcode does when generating the `ActionTestMetadata` identifier. Exactly how to do this is undocumented, so folks have attempted to figure it out empirically. Most recently, dda4007c7a ([trainer] fix issues where number of failures would always be zero (#21432), 2024-01-10) changed this transformation code in a way that fixed test names that contain spaces, but broke Objective-C test names. This commit fixes the regression caused by dda4007c7a with ObjC tests as follows: 1) It reverts the transformation behavior to how the code worked previously. Instead of "sanitizing" both the identifier and the test case name, it now keeps the identifier as is and "normalizes" the test case name. The test case name normalization has been moved into its own method which passes unit tests for ObjC, Swift, and tests with spaces. 2) It adds an ObjC xcresult bundle and unit test to prevent regressing parsing ObjC test names. 3) It refactors summaries_to_data so that if the corresponding test failure message cannot be located (but hopefully that won't happen anymore), we at least properly report that the test failed, but use the generic "unknown failure message" as the failure message. See discussion on: - https://github.com/fastlane/fastlane/pull/21565 - https://github.com/fastlane/fastlane/pull/21432 --- trainer/lib/trainer/test_parser.rb | 10 ++-- trainer/lib/trainer/xcresult.rb | 41 +++++++-------- ...5UOgWiKPIxZQdQeP-BtGoBEwiEcoYSkz_QqfjjMA== | Bin 0 -> 2360 bytes ...ezMh3vFXEmz1umKNEzvzPaQgZfSqBlgcgIowxQdg== | Bin 0 -> 924 bytes ...5UOgWiKPIxZQdQeP-BtGoBEwiEcoYSkz_QqfjjMA== | Bin 0 -> 397 bytes ...ezMh3vFXEmz1umKNEzvzPaQgZfSqBlgcgIowxQdg== | Bin 0 -> 67 bytes .../fixtures/Test.objc.xcresult/Info.plist | 29 +++++++++++ trainer/spec/test_parser_spec.rb | 47 ++++++++++++++++++ 8 files changed, 99 insertions(+), 28 deletions(-) create mode 100644 trainer/spec/fixtures/Test.objc.xcresult/Data/data.0~QcTkA55UPcxEP1nO1AY1osK3wJ6ccJbYiM6K7BJmWZ0MFG5UOgWiKPIxZQdQeP-BtGoBEwiEcoYSkz_QqfjjMA== create mode 100644 trainer/spec/fixtures/Test.objc.xcresult/Data/data.0~X7J2zgrHmeOeXrqjN4i9NpJunCOzuFwvG8Jap1VZxvIbhWezMh3vFXEmz1umKNEzvzPaQgZfSqBlgcgIowxQdg== create mode 100644 trainer/spec/fixtures/Test.objc.xcresult/Data/refs.0~QcTkA55UPcxEP1nO1AY1osK3wJ6ccJbYiM6K7BJmWZ0MFG5UOgWiKPIxZQdQeP-BtGoBEwiEcoYSkz_QqfjjMA== create mode 100644 trainer/spec/fixtures/Test.objc.xcresult/Data/refs.0~X7J2zgrHmeOeXrqjN4i9NpJunCOzuFwvG8Jap1VZxvIbhWezMh3vFXEmz1umKNEzvzPaQgZfSqBlgcgIowxQdg== create mode 100644 trainer/spec/fixtures/Test.objc.xcresult/Info.plist diff --git a/trainer/lib/trainer/test_parser.rb b/trainer/lib/trainer/test_parser.rb index c973043d290..2d1209948a5 100644 --- a/trainer/lib/trainer/test_parser.rb +++ b/trainer/lib/trainer/test_parser.rb @@ -267,19 +267,17 @@ def summaries_to_data(summaries, failures, output_remove_retry_attempts: false) end info[:retry_count] = retry_count - # Set failure message if failure found - failure = test.find_failure(failures) - if failure + case test.test_status + when "Failure" test_row[:failures] = [{ file_name: "", line_number: 0, message: "", performance_failure: {}, - failure_message: failure.failure_message + failure_message: test.failure_message(failures) }] - info[:failure_count] += 1 - elsif test.test_status == "Skipped" + when "Skipped" test_row[:skipped] = true info[:skip_count] += 1 else diff --git a/trainer/lib/trainer/xcresult.rb b/trainer/lib/trainer/xcresult.rb index 934fabd97a2..7f6a64a6fc6 100644 --- a/trainer/lib/trainer/xcresult.rb +++ b/trainer/lib/trainer/xcresult.rb @@ -172,29 +172,11 @@ def all_subtests return [self] end - def find_failure(failures) - sanitizer = proc { |name| name.gsub(/\W/, "_") } - sanitized_identifier = sanitizer.call(self.identifier) - if self.test_status == "Failure" - # Tries to match failure on test case name - # Example TestFailureIssueSummary: - # producingTarget: "TestThisDude" - # test_case_name: "TestThisDude.testFailureJosh2()" (when Swift) - # or "-[TestThisDudeTests testFailureJosh2]" (when Objective-C) - # Example ActionTestMetadata - # identifier: "TestThisDude/testFailureJosh2()" (when Swift) - # or identifier: "TestThisDude/testFailureJosh2" (when Objective-C) - - found_failure = failures.find do |failure| - # Sanitize both test case name and identifier in a consistent fashion, then replace all non-word - # chars with underscore, and compare them - sanitized_test_case_name = sanitizer.call(failure.test_case_name) - sanitized_identifier == sanitized_test_case_name - end - return found_failure - else - return nil + def failure_message(failures) + found = failures.find do |failure| + self.identifier == failure.normalized_name end + return found ? found.failure_message : "unknown failure message" end end @@ -385,6 +367,21 @@ def initialize(data) super end + def normalized_name + # Return test_case_name normalized to match the identifier from ActionTestMetadata + # Example TestFailureIssueSummary: + # producingTarget: "TestThisDude" + # test_case_name: "TestThisDude.testFailureJosh2()" (when Swift) + # or "-[TestThisDudeTests testFailureJosh2]" (when Objective-C) + # Example ActionTestMetadata + # identifier: "TestThisDude/testFailureJosh2()" (when Swift) + # or identifier: "TestThisDude/testFailureJosh2" (when Objective-C) + if self.test_case_name.start_with?("-[") && self.test_case_name.end_with?("]") + return self.test_case_name[2...-1].tr(" ", "/") + end + return self.test_case_name.tr(".", "/") + end + def failure_message new_message = self.message if self.document_location_in_creating_workspace&.url diff --git a/trainer/spec/fixtures/Test.objc.xcresult/Data/data.0~QcTkA55UPcxEP1nO1AY1osK3wJ6ccJbYiM6K7BJmWZ0MFG5UOgWiKPIxZQdQeP-BtGoBEwiEcoYSkz_QqfjjMA== b/trainer/spec/fixtures/Test.objc.xcresult/Data/data.0~QcTkA55UPcxEP1nO1AY1osK3wJ6ccJbYiM6K7BJmWZ0MFG5UOgWiKPIxZQdQeP-BtGoBEwiEcoYSkz_QqfjjMA== new file mode 100644 index 0000000000000000000000000000000000000000..bde6eb37acb1a8bb030af758b51368ab9b1f2faa GIT binary patch literal 2360 zcmV-83CH#*wJ-euNPS5FN{xUTF3@D?YB2PxBuh&`H9D~G z)KNg>(PFC}<$TRPe$8Keq4^7UJFn!Nb1F9aI@=264bz+P;HxvV(d?!j^m#)BY_F;I z`1=jA{~0LL4J1H*j(U=fW;dNk76kA`#nGJKb4yOnR*NZvTmN~o-#J%b$m#KjJ#e;K zOe4I462o-2Gq!bgXBOM~3-Z;dr?5oP!&jqe_xS{~XY+X# z5@L~(+-iAggKEdrey5Fwn3ppd`@j1%Ts0F_zKfaCyi=(4jyx0Ok z3iRFgF+4(Br6niSc3;Q-G{5AGw3WDoJ$w#OtYrKDu)H%ay;eK_uy?~(`wM=(Kkqs^ zs@={PP|gE;tETwvbI^t*0|8U+y*4?1D>h zYD_g2z(Oj3KjZC$w{^nVx>-m7Q33mt=PbS8*BZri;{d=@WSu`FU}@E8ZdO1bb2e;q8)M@L7g=xD@~MWSUn{&WBf!ewl+W181wIj~!Mr(e}Nq-OVXU&YZxb z%v_3ica=tTe(g~#P=NC)UnfBqyfL0>=$$_GY4IcMY$gRn|<3F511x4oL*-pyAh33F&o z5fViMXi`+rrAbjipQwcNIq~*xz8Wh6CKx&F?rr#lF&vNx6{#+25g&;dO^n0?Qd1&Q zIUorKgqk3YiQ$zLVGq;=<(yY#MailB0h2;8Dj5a=xfm4#!xTq9AUF-yg>onh8pEJ$ z2yH-k2n_`k15spwUXg|lV{&m+8!b!t$v4!EV;c>$gD~k3E>W`JY#IZPi-Vin5oloU}4i?Lb8*45qc3F`crPZ$-k$yz9+$ghY_ z7GVucu8k6m*`G{chRK2g4}yV{2Bn}`ih}ysG>%5W7-S+PuuvWt4kM9Xr6O1l1glC= z$U2nF$STJIDLXu5lU9R*4V%;Mi=~}K=g&5U1rb-mXcyWg>r=s22cYG zAU}u$L<8t*sG}R=p##dmz$+@6q6xzu=A2O0`LlXKkJ+y}c+cx1-jZXl=Bz@LVpC0nsil1Bn zoT>65f7&Ap9H&GMAPND?!GHLRNd)bAT!jW?P*w39fPL6A#6b!|bWj0M*z=J+qa`K; zGvYDzjWqgzNn`;B3s8Q5JgY)j#rEjZ2tis6%5BtM+|!13{%0-YN3VP<$=~M|Xo5vv z?O=j9r>-|MOcVsUL(`Zsp#1cS3r zd&<@~4}pJtWXSF$3{A;dnjbYf!@AV-$R(PA-x~tX*2fbSk=qt17Y*~XGZW44qD>Ou zz94$O?;zLgczp|`wlI6CCT%#*dH2(?jbythb6-@PaAYEH+UX*|>iB%+FHZ)kon*N# zkI~YY&j_w_l%#TXM)3Dct8R;)aSl?0DDbY8bM#|~ER4r%WCfV}Os28qyKKM|h6J7{3Ns4Z65W0kLlI%+io4URJ2@q@BN$S!extI<=Mnmne8oz#c(nY5`6@|DJ5Hqv@iWIk$R(QQ8KAN**0=vz}We@TPr e4+&1u8%Yrc<+|Nr9IPXr%q>U=K$A6h)bTwlfMIk1 literal 0 HcmV?d00001 diff --git a/trainer/spec/fixtures/Test.objc.xcresult/Data/data.0~X7J2zgrHmeOeXrqjN4i9NpJunCOzuFwvG8Jap1VZxvIbhWezMh3vFXEmz1umKNEzvzPaQgZfSqBlgcgIowxQdg== b/trainer/spec/fixtures/Test.objc.xcresult/Data/data.0~X7J2zgrHmeOeXrqjN4i9NpJunCOzuFwvG8Jap1VZxvIbhWezMh3vFXEmz1umKNEzvzPaQgZfSqBlgcgIowxQdg== new file mode 100644 index 0000000000000000000000000000000000000000..cf1d9746cc19c4c7857c2d6f1b1377b3f4178033 GIT binary patch literal 924 zcmV;N17rLswJ-euNSz!2rg4@h@XTtcB1HcH1n4#L&jHgezOjynijNWA(p=U6TZK`= zsu_3&p7#QY0EqyH0Mlzc1|pO{`}Nr`42HLd4a5AP;j=8BRRs^;-a5XJZ13ky)6|RU zD8gu29J|!d{yo8pYTJG50PxH=)<KS5weoLZV|uHQL2*we+lN9aWTiaLuyL`s~-+ zia%c*QkLjgd7?-?6=O$Bxo1VQUtF`Sv$T$)Rc!LvFKxST9WA4SCeg97T$WDx?B5So zw4bJS8m&q``*(yD#rWrK1Hf^$AH-<4`t0A-y9U?q2P?|iFz1-fb{0|Ge4Y5j{far| zt#dZa^@J6j`LkaP$85H^E~?bIk1qtSI!4K#;cGuej6P zjJ7=>Ng$~K87T*p6MAo80D4S<2$C8Tgklu_{TfcnF)pheg3tbpMq`T7oAE&yrTHQV zAB>Iq6;t06KKn(%X_`>JC*PdXn@@^Tim1>2wzJ5e{gN?)6UvB+BspR9EbG$crvAkM z40AB11i&yy&j%|iqZnT!#wn+aCK*?5~+aN75O;t3$bg|7thm5a&EsN09Se0s<6Q@`$)*jMf`ImsH_o9GUP| z5Z$E1$?PSmY_dTF;xegdpBw=%nc-3(9+{ODh>a7FRVYCCY#i3-laWj!HU<%g8v&`* ziq~)6jumyQIrfZGdS;yBc@THltp${nCTW_Ul;)#m<3-CtY0hqKXZ4i3uWx-0bEt8l z1Tqs$Fu?%K2w@8tU;+qWsO<KrjekahxF`1OiEj5QUIYWJDw=2`VTlOm(~|qoHqv zVkA)l0TD=WwB{wV8_`M(e!{^QHlPrV?(#g6*x&U)UiCOr!n41Ha!a}d4_E?~t+&7{ zd|(WldSJ1+*?t^GJvlKOL!dKj`cIVr*n-EuR1hMk3_$=I8Y!JOwtsUVL{NCP#aPD&Y0Tbp`Mvcni^K`;ngD((pz5!+Ll yw-{rRI%!9Pg7I&~&b!#K7Y!Do%3F#=`bjC7mB`r|>=T?SX&g9~eA)Tdfa3SGq`Kt* literal 0 HcmV?d00001 diff --git a/trainer/spec/fixtures/Test.objc.xcresult/Data/refs.0~QcTkA55UPcxEP1nO1AY1osK3wJ6ccJbYiM6K7BJmWZ0MFG5UOgWiKPIxZQdQeP-BtGoBEwiEcoYSkz_QqfjjMA== b/trainer/spec/fixtures/Test.objc.xcresult/Data/refs.0~QcTkA55UPcxEP1nO1AY1osK3wJ6ccJbYiM6K7BJmWZ0MFG5UOgWiKPIxZQdQeP-BtGoBEwiEcoYSkz_QqfjjMA== new file mode 100644 index 0000000000000000000000000000000000000000..5a4fc51debe4ea3278f09ebc3dc4a072001a4520 GIT binary patch literal 397 zcmV;80doEZK>!J?i)+o?PHO$DQB%R(Q4D9@lhQUsnM)@2 zm^a@e8EDu*=gx-n%3|$%yHSAy^d*!~fi|60FB0anRz7$=pMbOVv>(jY8J1 z;;Fe>$l7`f8hjJKK>&osyI7YH_@+?7Ay_!>*o)1@(7Mn2y`-k~X%^x|@tJ!uBi3n4 zOq;cpzJ1e8@cRy$Stl~2y4%pGjXN{XCLKWl$l+VYs)4G4vBADGGY&T-_&Isg=0sAy rtfyK@g#a)|VXYc7u{~}&%rh*@;pHg_FyFJkkOj$rRTVnW9CX3Xbc_SYb5Az-xJ ZnmkXra-!GLE3!5@Pc + + + + dateCreated + 2024-04-23T23:38:03Z + externalLocations + + rootId + + hash + 0~QcTkA55UPcxEP1nO1AY1osK3wJ6ccJbYiM6K7BJmWZ0MFG5UOgWiKPIxZQdQeP-BtGoBEwiEcoYSkz_QqfjjMA== + + storage + + backend + fileBacked2 + compression + standard + + version + + major + 3 + minor + 39 + + + diff --git a/trainer/spec/test_parser_spec.rb b/trainer/spec/test_parser_spec.rb index af977270a72..130a3ed92b8 100644 --- a/trainer/spec/test_parser_spec.rb +++ b/trainer/spec/test_parser_spec.rb @@ -245,6 +245,53 @@ } ]) end + + it "works as expected with Objective C xcresult", requires_xcode: true do + tp = Trainer::TestParser.new("./trainer/spec/fixtures/Test.objc.xcresult") + expect(tp.data).to eq([ + { + project_path: "HelloWorldTest.xcodeproj", + target_name: "HelloWorldTestTests", + test_name: "HelloWorldTestTests", + configuration_name: "Test Scheme Action", + duration: 0.04512190818786621, + tests: [ + { + identifier: "HelloWorldTestTests.testExample", + name: "testExample", + duration: 0.0011409521102905273, + status: "Success", + test_group: "HelloWorldTestTests", + guid: "", + }, + { + identifier: "HelloWorldTestTests.testFailure", + name: "testFailure", + duration: 0.043980956077575684, + status: "Failure", + test_group: "HelloWorldTestTests", + guid: "", + failures: [ + { + failure_message: "((NO) is true) failed - Fail (/Users/jsoffian/HelloWorldTest/HelloWorldTestTests/HelloWorldTestTests.m#CharacterRangeLen=0&EndingLineNumber=39&StartingLineNumber=39)", + file_name: "", + line_number: 0, + message: "", + performance_failure: {} + } + ] + } + ], + number_of_tests: 2, + number_of_failures: 1, + number_of_skipped: 0, + number_of_tests_excluding_retries: 2, + number_of_failures_excluding_retries: 1, + number_of_retries: 0 + } + ]) + end + end end end