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

nixos/tests/phosh: init #173108

Merged
merged 2 commits into from Nov 19, 2022
Merged

nixos/tests/phosh: init #173108

merged 2 commits into from Nov 19, 2022

Conversation

zhaofengli
Copy link
Member

Description of changes

Makes Phosh updates easier to test.

Screenshots in $out

01lockinfo
02unlock
03launcher
04settings
05keyboard

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@zhaofengli
Copy link
Member Author

@ofborg test phosh

phone.screenshot("01lockinfo")

with subtest("Check that we can unlock the screen"):
phone.send_chars("${pin}", delay=0.2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider leaving a comment to explain why a longer delay is needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure thing. I'm not entirely sure what the cause of the jankiness is, but I suspect it has something to do with the animations.

Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if the delay kwarg is not used?

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems to me like it defaults to 0.01s by default

Copy link
Contributor

@tomfitzhenry tomfitzhenry left a comment

Choose a reason for hiding this comment

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

LGTM with a few minor comments.

@tomfitzhenry
Copy link
Contributor

@tfc, friendly ping! This would be a really useful test for some in-progress PRs: #183913 and #183907.

@tfc
Copy link
Contributor

tfc commented Aug 2, 2022

@zhaofengli LGTM. can you please rebase?

@zhaofengli
Copy link
Member Author

Rebased and tested with #183913. It worked with the Phoc update in #183907, but with the Phosh update applied it crashed with "No schemas installed".

@tfc
Copy link
Contributor

tfc commented Aug 2, 2022

@ofborg test phosh

@zhaofengli
Copy link
Member Author

The test doesn't work now before #183913 is merged.

@tfc
Copy link
Contributor

tfc commented Aug 2, 2022

ok didn't get that before. So let's get that merged in first.

@tomfitzhenry
Copy link
Contributor

#183913 has now been merged.

@zhaofengli
Copy link
Member Author

@ofborg test phosh

@tfc
Copy link
Contributor

tfc commented Aug 4, 2022

It seems like the test ran through on arm but didn't build on x86. can you please have a look @zhaofengli ?

@zhaofengli
Copy link
Member Author

Hmm, looking at the logs there seem to be a huge number of rebuilds which I didn't encounter locally, and it failed building an unrelated package:

25/29 test-shortcut-theme               OK              0.24s
26/29 test-model-filter                 OK              0.08s
27/29 test-fuzzy-highlight              OK              0.05s
28/29 test-mutable-scoring              OK              0.05s
29/29 test-recursive-monitor            OK              0.15s

Summary of Failures:

24/29 test-suggestion-buffer    FAIL            0.27s   killed by signal 5 SIGTRAP


Ok:                 28  
Expected Fail:      0   
Fail:               1   
Unexpected Pass:    0   
Skipped:            0   
Timeout:            0   

Full log written to /build/libdazzle-3.44.0/build/meson-logs/testlog.txt
TEST    vsynth2-huffyuvbgr24
TEST    vsynth2-huffyuvbgra
error: builder for '/nix/store/2kydag4y4j1qk1la663rdvsvg0jywiy9-libdazzle-3.44.0.drv' failed with exit code 1;

@tfc
Copy link
Contributor

tfc commented Aug 5, 2022

@ofborg test phosh

@tomfitzhenry
Copy link
Contributor

tomfitzhenry commented Aug 7, 2022

This is passing:

This is failing on aarch64 ofborg: https://logs.nix.ci/?key=nixos/nixpkgs.173108&attempt_id=455cc027-e508-480b-9d56-786702b17dd7

Screenshots in `$out`

01lockinfo
02unlock
03launcher
04settings
05keyboard

@tfc
Copy link
Contributor

tfc commented Aug 8, 2022

This is passing:

* on x86_64 ofborg: https://github.com/NixOS/nixpkgs/pull/173108/checks?check_run_id=7687825361

* locally (tested via `nix-build . -A nixosTests.phosh`).

This is failing on aarch64 ofborg: https://logs.nix.ci/?key=nixos/nixpkgs.173108&attempt_id=455cc027-e508-480b-9d56-786702b17dd7
Screenshots in $out

Ok, this time the ofborg error is related to this actual test... as long as this is supposed to work on aarch64, we gonna get this test fixed, regardless if it works on some other machine. In the future it will has to be solid in the CI, too. Any chance that there might be some big-noisy-machine-multicore-flakiness?

@tomfitzhenry
Copy link
Contributor

tomfitzhenry commented Aug 9, 2022

I ran the test on my aarch64-linux pinebook-pro. The first hurdle that qemu seems to fail on big.LITTLE architecture. To workaround that, disable the little cores: cd /sys/devices/system/cpu/; for i in cpu{0,1,2,3}; do echo 0 | sudo tee $i/online; done. This isn't needed on ofBorg (which likely doesn't have a big.LITTLE architecture), as we observe that ofBorg is able to run qemu on aarch64.

Then when I run the test, I'm able to reproduce a failure, but it looks different to ofBorg's:

Screenshots in `$out`

01lockinfo
02unlock
03launcher

The test log showed phoc segfaulted.

I'll keep looking.

@tomfitzhenry
Copy link
Contributor

If I remove "WLR_RENDERER" = "pixman"; from the phosh.nix test, then this test passes on my pinebook-pro.

Screenshots in `$out`

01lockinfo
02unlock
03launcher
04settings
05keyboard

I will re-run this a few times to check it's consistent.


systemd.services.phosh = {
environment = {
# Accelerated graphics fail on phoc 0.20 (wlroots 0.15)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, it sounds like you've tested this against #183907 (or similar) and found that the test would fail?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was testing with #183907 and without using the software renderer it failed saying "Could not find needed globals" with outputs being NULL:

phone # [  118.480569] systemd[838]: Starting Phosh, a shell for mobile phones...
phone # [  118.746481] .phosh-wrapped[4390]: Could not find needed globals
phone # outputs: 0, layer_shell: 0x118d100, idle_manager: 0x1196510, inhibit: 0x1196490, xdg_wm: 0x118d000, xdg_output: 0x118d080, wlr_output_manager: 0x1142400, wlr_foreign_toplevel_manager: 0x114f740[  118.816159] traps: .phosh-wrapped[4390] trap int3 ip:4a044f sp:7fffda8b42c0 error:0 in .phosh-wrapped[419000+c1000]

@tomfitzhenry
Copy link
Contributor

I will re-run this a few times to check it's consistent.

It wasn't consistent, and per "Accelerated graphics fail on phoc 0.20 (wlroots 0.15)" it sounds like this test is sensitive to phoc 0.17 (status quo) vs phoc 0.20 (upcoming: #183907).

In which case, I expect it's preferable to do the phoc 0.20 upgrade (with manual testing), then add this test.

@tomfitzhenry
Copy link
Contributor

In #199819, I managed to get the phosh test passing on @ofborg by removing the two final subtests. The first 3 subtests offer the most value, so I suggest removing the final two subtests from this PR, and that this PR is merged. We can iterate on it as needed.

@tomfitzhenry
Copy link
Contributor

@ofborg test phosh

Copy link
Contributor

@tomfitzhenry tomfitzhenry left a comment

Choose a reason for hiding this comment

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

LGTM. Tests are passing on @ofborg.

@Mindavi Mindavi merged commit 3d18556 into NixOS:master Nov 19, 2022
@Mindavi
Copy link
Contributor

Mindavi commented Nov 19, 2022

I like this! Great job adding this :)

@tomfitzhenry tomfitzhenry mentioned this pull request Nov 19, 2022
13 tasks
@zhaofengli zhaofengli deleted the phosh-test branch November 20, 2022 03:57
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.

None yet

6 participants