keyman
keyman copied to clipboard
keymanapp
fix(web): button, float init timer cleanup
Cleans up some code that's proving problematic for some of our unit tests; there's some cross-UI-module pollution happening on occasion that turns into a race condition of sorts.
@keymanapp-test-bot skip
User Test Results
Test specification and instructions
User tests are not required
Test Artifacts
- Android
- Developer
- iOS
- Keyboards
- Web
- Windows
![keymanapp-test-bot[bot] avatar](https://avatars.githubusercontent.com/in/133554?v=4 "Published by a pub.dev verified publisher"){kind=small}
... and the child PR (#7037) had a test failure that I thought this one would've fixed. Looks like there may be another aspect or two to investigate.
With the latest commit, I've re-run the test suite multiple times and no longer see the test failure that sent this back to draft. 4 successes on one build agent, 2 on another.
I'm trying to verify this for the build agent (pp-602) that actually produced that error as well, but it's having trouble actually getting the full test suite to run at this time.
... Huh. Build agent pp-602 is almost-consistently failing to support a BrowserStack connection to a Safari test environment target. The other build agents seem fine; it's just pp-602 that's having this problem. It just... won't "capture" in time, according to the logs. And mostly for that one test-environment at that.
... Huh. Build agent
pp-602is almost-consistently failing to support a BrowserStack connection to a Safari test environment target. The other build agents seem fine; it's justpp-602that's having this problem. It just... won't "capture" in time, according to the logs. And mostly for that one test-environment at that.
I've just checked the browserstack-local executable version on both Windows agents and they appear to be identical. That said, the version available for download from browserstack.com differs (it's a different file size even though the file version is the same?!)
I can back up the existing executables on the agents and replace with the new version from the downloads site. Don't know if that will make a difference or not, of course; suspect not.
UPDATE: we have version 8.6 (metadata on the executable is wrong -- reported to BrowserStack). The latest version is 8.7. Given that, I will update the version on the build agents once my test builds finish running.
Here's what I am seeing for the web test build history. I am not seeing the same correlation that you report. It's possible that there are odd real failing tests in this log but most of these builds have no web changes.
And the same log for LM layer. Again, I don't see a real correlation between agent and failing build.
I'm going to re-run the build for the latest commit on this PR on all agents and see what emerges. Results:
- ba-win10-64-pp-602 web: Tests passed: 811, ignored: 6; exit code 1
- ba-win10-64-s1-601 web: Tests passed: 969, ignored: 7
- ba-bionic64ta web: Tests passed: 969, ignored: 7
LMLayer:
- ba-win10-64-pp-602 lmlayer: Tests passed: 172, ignored: 1; exit code 1
- ba-win10-64-s1-601 lmlayer: Tests passed: 181, ignored: 1
UPDATE: we have version 8.6 (metadata on the executable is wrong -- reported to BrowserStack). The latest version is 8.7. Given that, I will update the version on the build agents once my test builds finish running.
Version 8.7 made no difference. So ... pp_602 has a slightly different performance profile than s1_601, so perhaps there's still another race somewhere?
Here's what I am seeing for the web test build history. I am not seeing the same correlation that you report. It's possible that there are odd real failing tests in this log but most of these builds have no web changes.
[omitted image]
And the same log for LM layer. Again, I don't see a real correlation between agent and failing build.
[omitted image]
Unfortunately, those images are irrelevant. Please remember that this PR is making other changes that fix issues underlying what you've observed here.
In my test runs so far, this PR fully stabilizes KMW test builds for all agents but pp-602 - when you include its changes. The other branches in the images above don't have these "other changes", so of course there are still problems on other agents there - this PR's fixes haven't landed yet!
I'm going to re-run the build for the latest commit on this PR on all agents and see what emerges. Results:
ba-win10-64-pp-602 web: Tests passed: 811, ignored: 6; exit code 1
ba-win10-64-s1-601 web: Tests passed: 969, ignored: 7
ba-bionic64ta web: Tests passed: 969, ignored: 7
LMLayer:
ba-win10-64-pp-602 lmlayer: Tests passed: 172, ignored: 1; exit code 1
ba-win10-64-s1-601 lmlayer: Tests passed: 181, ignored: 1
That looks like the "same correlation" to me. Note the lower "tests passed" number (and in the case of the first set, the lower 'ignored' number) and the fact that no tests were marked as failing. Those are the hallmark symptoms of that BrowserStack "failure to capture" problem.
UPDATE: we have version 8.6 (metadata on the executable is wrong -- reported to BrowserStack). The latest version is 8.7. Given that, I will update the version on the build agents once my test builds finish running.
Version 8.7 made no difference. So ... pp_602 has a slightly different performance profile than s1_601, so perhaps there's still another race somewhere?
Even if "we" do, it's wholly external to our code. Our unit tests aren't even "spun up" at that level.
Even if "we" do, it's wholly external to our code. Our unit tests aren't even "spun up" at that level.
Righto. Where we are at now, we still have unstable tests. That's not... great. Any suggestions on where to go now? pp-602 and s1-601 are essentially identical VMs in the same datacenter. So it's hard to see what could be causing this. I mean, I can temporarily disable KeymanWeb and Common-LMLayer building on pp-602 but that's going to slow down builds and obviously isn't really a 'solution' so much as a bandaid.
