hey24sheep / azure-flutter-tasks

Easily build and deploy with latest Flutter build tasks for Azure DevOps Pipelines Tasks
https://marketplace.visualstudio.com/items?itemName=Hey24sheep.flutter
MIT License
89 stars 22 forks source link

Unhandled: Cannot read property 'failed' of null #83

Closed alex1001xela closed 1 year ago

alex1001xela commented 1 year ago

Hey there! I am trying to run the flutter test task and I get the error in the title. When I run the tests locally, I get no errors at all, they seem fine. I use locally the same sdk version as on the server. Running the tests on the server as a simple powershell command also works.

Any ideas?

This is the log:

2023-06-13T14:01:17.9632489Z ##[section]Starting: Test
2023-06-13T14:01:17.9762196Z ==============================================================================
2023-06-13T14:01:17.9762546Z Task         : Flutter Test Task
2023-06-13T14:01:17.9762819Z Description  : Executes all tests for a Flutter project.
2023-06-13T14:01:17.9763047Z Version      : 0.3.0
2023-06-13T14:01:17.9763229Z Author       : Hey24sheep
2023-06-13T14:01:17.9763597Z Help         : For more information, take a look at the Flutter [documentation](https://flutter.io)
2023-06-13T14:01:17.9764180Z ==============================================================================
2023-06-13T14:01:20.2510285Z [command]C:\Windows\system32\cmd.exe /D /S /C "C:\___\_work\_tool\Flutter\3.10.4\windows\flutter\bin\flutter.bat test --pub"
2023-06-13T14:01:41.0667508Z ##[error]Unhandled: Cannot read property 'failed' of null
2023-06-13T14:01:41.0679905Z 
2023-06-13T14:01:41.1252595Z ##[error]Unhandled: Cannot read property 'failed' of null
2023-06-13T14:01:41.1256105Z 
2023-06-13T14:01:41.1653494Z ##[error]Unhandled: Cannot read property 'failed' of null
2023-06-13T14:01:41.1656098Z 
2023-06-13T14:01:42.4142181Z ##[section]Async Command Start: Publish test results
2023-06-13T14:01:42.4815595Z Publishing test results to test run '466'
2023-06-13T14:01:42.5952591Z Published Test Run : http://___/_TestManagement/Runs?runId=466&_a=runCharts
2023-06-13T14:01:42.6533971Z ##[section]Async Command End: Publish test results
2023-06-13T14:01:42.6535772Z ##[section]Finishing: Test
hey24sheep commented 1 year ago

Hi, it's weird that you are getting this error. I have never seen this error and somewhere a null object is thrown and it's trying to access the property of that.

Test Tasks hasn't been updated for past 1-2 years (I think). Anyone else getting this error?

fresnault commented 1 year ago

I have exactly the same thing, only on windows agent (2022 in my case). On macOS and ubuntu it works fine.

Version      : 0.3.0
Author       : Hey24sheep

C:\Windows\system32\cmd.exe /D /S /C "C:\hostedtoolcache\windows\Flutter\3.10.5\windows\flutter\bin\flutter.bat test --pub --coverage"

...

##[error]Unhandled: Cannot read property 'failed' of null
##[error]Unhandled: Cannot read property 'succeeded' of null
hey24sheep commented 1 year ago

I see, so it could be from Flutter's side maybe. If it isn't failing on other OS agents.

Could anyone of you open this issue on Flutter's repo?

Adam-Langley commented 1 year ago

Hi,

I am also getting this issue. It only occurs on Windows build agents.

I've tracked it down to this line :

https://github.com/hey24sheep/azure-flutter-tasks/blob/18d4947ae5c11bc94ae8f71e29284b0e2de38a95/tasks/test/index.ts#L147C1-L148C1

the 'suite' variable is null.

I'm guessing that it's because the code does not ensure the initialization of currentSuite before beginning the parsing of the STDOUT text https://github.com/hey24sheep/azure-flutter-tasks/blob/18d4947ae5c11bc94ae8f71e29284b0e2de38a95/tasks/test/index.ts#L128C13-L128C27

Probably just null-test the currentSuite variable on line 128 before invoking createTestCase?

I expect it's just that the STDOUT messages produced by flutter on Windows is slightly different than on POSIX systems.

Adam-Langley commented 1 year ago

Here's a PR to try out - I've done it totally blind - just to illustrate where I think the issue is...

https://github.com/hey24sheep/azure-flutter-tasks/pull/90

hey24sheep commented 1 year ago

@Adam-Langley thank you for your PR, really appreciate it.

Please check and let me know if this issue is fixed (so I can close this issue) on latest v0.3.13. Extension is already published on the marketplace.

Adam-Langley commented 1 year ago

Thanks for that. I'll give it a try when I'm back in the office Monday.

Adam-Langley commented 1 year ago

Hi @hey24sheep

Unfortunately - this blind fix wasn't enough :(

It's no longer producing an unhandled error - the task completes, however it fails to collect any results (on Windows).

I've dug into the verbose logging, and compared between Windows and iOS - the big difference, is that on Windows the message "00:01 +0: loading /Users/runner/work/1/s/src/app_name/test/XXX/YYY_test.dart" is never produced. Very odd that Google is writing out different log messages on different platforms... so we need to either handle them differently, or use a message that is common to all platforms.

Here is a message that exists for them all... 00:23 +0: /Users/runner/work/1/s/src/app_name/test/XXX/YYY.dart: (setUpAll)

I've confirmed that message is generated at the beginning of each test for Ubuntu, Mac and Windows.

If you are happy to do another blind release (it's probably less work that me setting up an MS marketplace account, etc, etc) - then I'll put together another PR on Monday.

hey24sheep commented 1 year ago

@Adam-Langley I see, it is weird that they are outputting different logs. I think issue should be opened up on Flutter repo. Maybe they have a inconsistency bug that they don't know.

Thanks for debugging it further. I don't have access to Azure anymore so I can't test these things.

I'll do a release during this coming week as soon as I get a little time.

Adam-Langley commented 1 year ago

Hi @hey24sheep - another blind offering if you want to push it when you have time, I can test it.

hey24sheep commented 1 year ago

Hi all, I have updated the extension to 0.3.14 it introduces a new separator input param to fix the multiple arguments or multiple dart defines (containing whitespace within values). Please check the readme & changelogs for further details.

Builds must be fixed now, please let me know by closing your respective issues, thank you all.

hey24sheep commented 1 year ago

@Adam-Langley I have merged the change you made. Can you please verify it on Windows as well as on Ubuntu. It will be very helpful for me. Otherwise we will need to add previous regex along with the current one to fix this issue, maybe, thanks a bunch.

Adam-Langley commented 1 year ago

Fantastic, thanks, I'll test this out today

Adam-Langley commented 1 year ago

Hi @hey24sheep, Something's not right... these changes havent broken anything, but the "suites" collection is still [] on Windows after running the tests. So, no need to reverse anything, but there is clearly something else going on here that is preventing the suite detector regex that we've been editing, from picking up the message on Windows. I suspect it's going to be something like, these messages are coming from STDERR rather than STDOUT on Windows, and so both need to be redirected... or a newline (CRLF) issue preventing the STDOUT buffer from flushing in the same way - something platform specific.

For example: You're monitoring raw 'STDOUT', and treating it like lines...

    testRunner.on('stdout', line => {

but I believe there is a proper event for processed lines

    testRunner.on('stdline', line => {

So maybe using this would be better?

I expect this is going to need debugging, rather than doing this over and over...

I'll see if I can set up an environment - but can't do so right now. Thanks for the help and quick responses - let's leave this open please and I'll post an update when I've managed look at it. Perhaps someone else has one already set up and with all this information, can address it before me.

Anyway, I'll keep you posted. Thanks,

hey24sheep commented 1 year ago

Hi @Adam-Langley I have pushed a new update 0.3.15 this should fix the error properly. It's not about stdline or stderr or stdout. The code itself didn't make sense so I have updated it to consider both regex otherwise it just treats it as a single file run and add tests cases to that single autogenerated namespace.

There were actually two errors, one was publish must be failing on this null or in cases where the title wasn't there. So, new push will fix that too.

You can test whenever you can and let me know, that will be helpful. Thanks.

Adam-Langley commented 1 year ago

Hi @hey24sheep Thanks for digging in to that. I think you're almost there.

The task is now reporting results on Windows - however, they all (Windows/Linux/Mac) are reporting hundreds of repeated test results - i.e. (I currently only have about a dozen unit tests in this build)

image

I think the root cause is going to be here:

image

It looks like the code is adding the 'currentSuite' to the suites collection over and over.

There is a lot of unnecessary data in this data structure. I've attached a dump of the results json from my build server.

unit_tests_results.json.zip

Thanks

hey24sheep commented 1 year ago

@Adam-Langley Hi, can you test on latest 0.3.16 I have re-written the whole logic for test suite and case creation.

alex1001xela commented 1 year ago

I think it works! Just did two runs and they pass. Thank you all for your help! I hope it also worked for @fresnault . :)

hey24sheep commented 1 year ago

I will keep this open for a week otherwise close it

Adam-Langley commented 1 year ago

Hi @hey24sheep,

sorry for the late reply. It's definitely improved, but not quite there - I get inconsistent outputs across the 3 platforms. Same codebase, iOS/Android/Windows show 10 tests, Mac shows 8 - will provide more details once I've had a chance to dig in deeper.

hey24sheep commented 1 year ago

Linked issue https://github.com/hey24sheep/azure-flutter-tasks/issues/101

hey24sheep commented 1 year ago

I'm closing this issue as the original issue is solved but a new issue will be tracked in the the linked new one.

@Adam-Langley thank you, you can post your latest help on the new linked #101 issue. That would be helpful and great.