[autotests] Rewrite testShellClientRules
ClosedPublic

Authored by zzag on Jan 23 2019, 9:43 AM.

Details

Summary

Currently, the test doesn't verify that each rule does what it should,
e.g. a force rule is a force rule and not force temporarily, etc. This
as it turns out hides some bugs, e.g. all remember rules do not work,
forced window shortcuts can't be released, etc.

CCBUG: 403305

Diff Detail

Repository
R108 KWin
Lint
Lint ErrorsExcuse: ........
SeverityLocationCodeMessage
Errorautotests/integration/shell_client_rules_test.cpp:346CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:347CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:399CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:400CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:456CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:457CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:574CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:575CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:657CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:658CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:703CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:704CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:751CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:752CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:845CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:846CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:925CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:926CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:973CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:974CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:1021CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:1022CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:1114CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:1115CppcheckdoubleFree
Errorautotests/integration/shell_client_rules_test.cpp:1194CppcheckdoubleFree
Unit
No Unit Test Coverage
Build Status
Buildable 8988
Build 9006: arc lint + arc unit
zzag created this revision.Jan 23 2019, 9:43 AM
Restricted Application added a project: KWin. · View Herald TranscriptJan 23 2019, 9:43 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Jan 23 2019, 9:43 AM
zzag updated this revision to Diff 50178.Jan 24 2019, 11:35 AM

Add explanation for why testShortcutRemember, testShortcutForce, and
testShortcutForceTemporarily should be skipped.

zzag updated this revision to Diff 50184.Jan 24 2019, 1:47 PM

Verify the current virtual desktop in testDesktop*

zzag updated this revision to Diff 50185.Jan 24 2019, 1:52 PM

fix bool val

graesslin added inline comments.
autotests/integration/shell_client_rules_test.cpp
185

I don't really understand this. Before we had a test which tested the aspects already supported. For every added supported rule the test was added. Obviously this was not complete, it's not news that rules are fully implemented yet.

Now we have more test cases also for the things not yet supported, but the complete test is skipped. Thus also the already working aspects are not covered. I don't understand what the advantage is.

zzag updated this revision to Diff 50193.Jan 24 2019, 2:25 PM

Remove qskip.

zzag updated this revision to Diff 50424.Jan 28 2019, 12:08 PM

Don't test wl_shell.

zzag planned changes to this revision.EditedJan 31 2019, 10:22 PM

Once this change is rebased on David's change, I'll have to tweak createWindow helper.

zzag updated this revision to Diff 52804.Feb 28 2019, 12:12 PM
  • Rebase on master;
  • Use setOriginalSkipTaskbar instead of setSkipTaskbar for now;
  • KWin doesn't emit a configure event when interactive resize is finished, so mark corresponding QVERIFY and QCOMPARE with QEXPECT_FSIL.
zzag planned changes to this revision.Feb 28 2019, 12:58 PM
zzag edited the summary of this revision. (Show Details)

Make comments better.

davidedmundson added inline comments.
autotests/integration/shell_client_rules_test.cpp
1072

Can we expose the enums as public and use them here.

zzag added inline comments.Feb 28 2019, 1:23 PM
autotests/integration/shell_client_rules_test.cpp
1072

Yeah, it's a very good idea. Whould it be enough to expose only

// All these values are saved to the cfg file, and are also used in kstart!
enum {
    Unused = 0,
    DontAffect, // use the default value
    Force,      // force the given value
    Apply,      // apply only after initial mapping
    Remember,   // like apply, and remember the value when the window is withdrawn
    ApplyNow,   // apply immediatelly, then forget the setting
    ForceTemporarily // apply and force until the window is withdrawn
};

?

davidedmundson added inline comments.Feb 28 2019, 1:28 PM
autotests/integration/shell_client_rules_test.cpp
1072

And StringMatch

zzag updated this revision to Diff 52836.Feb 28 2019, 4:24 PM
  • Use enums;
  • Cleanup comments
davidedmundson accepted this revision.Feb 28 2019, 4:32 PM
This revision is now accepted and ready to land.Feb 28 2019, 4:32 PM
This revision was automatically updated to reflect the committed changes.