Project

General

Profile

Actions

action #107026

closed

coordination #109668: [saga][epic] Stable and updated non-qemu backends for SLE validation

coordination #100688: [epic][virtualization][3rd party hypervisor] Add svirt backend compatibility for vmware 7.0

Improve existing unit tests for VNC module to increase its test coverage (before doing any actual changes) size:M

Added by okurz over 2 years ago. Updated over 2 years ago.

Status:
Resolved
Priority:
High
Assignee:
Category:
Feature requests
Target version:
Start date:
2022-02-17
Due date:
% Done:

0%

Estimated time:

Description

Motivation

To be able to safely extend VNC which we plan for VMWare 7.0 support we need to cover consoles::VNC better with our test coverage

Acceptance criteria

  • AC1: 100% reported statement coverage for consoles::VNC in our CI tests
  • AC2: VNC protocol can be tested without sockets

Suggestions


Related issues 1 (1 open0 closed)

Related to openQA Project - action #76813: [tools] Test using svirt backend fails with auto_review:"Error connecting to VNC server.*: IO::Socket::INET: connect: Connection refused"New2020-10-30

Actions
Actions #1

Updated by okurz over 2 years ago

  • Tracker changed from coordination to action
Actions #2

Updated by okurz over 2 years ago

  • Priority changed from Normal to Low
Actions #3

Updated by kraih over 2 years ago

For some broader context, this document from the Python community explains pretty well why protocol implementations and I/O should be separated. https://sans-io.readthedocs.io/how-to-sans-io.html

Actions #4

Updated by okurz over 2 years ago

  • Status changed from New to Workable

We already estimated this ticket

Actions #5

Updated by livdywan over 2 years ago

  • Related to action #76813: [tools] Test using svirt backend fails with auto_review:"Error connecting to VNC server.*: IO::Socket::INET: connect: Connection refused" added
Actions #6

Updated by okurz over 2 years ago

  • Priority changed from Low to High

This is becoming more important for #99345 and #76813

Actions #7

Updated by mkittler over 2 years ago

  • Assignee set to mkittler
Actions #8

Updated by mkittler over 2 years ago

  • Status changed from Workable to In Progress
Actions #9

Updated by openqa_review over 2 years ago

  • Due date set to 2022-05-19

Setting due date based on mean cycle time of SUSE QE Tools

Actions #10

Updated by mkittler over 2 years ago

Actions #11

Updated by okurz over 2 years ago

merged. https://app.codecov.io/gh/os-autoinst/os-autoinst/blob/master/consoles/VNC.pm shows 87.56%, looking good, looking good :)

Actions #12

Updated by mkittler over 2 years ago

Last PR which should bring it to 100 %: https://github.com/os-autoinst/os-autoinst/pull/2051

(Not considering some other files and the C++ code. But we likely don't need to touch that and it is therefore out of scope.)


Split consoles::VNC into protocol and socket modules so that protocol is separated from networking in os-autoinst consoles::VNC

Note that I didn't follow that suggestion because for the sake of writing unit tests the current approach of mocking is sufficient. Since it is Perl any everything is dynamically typed we can always allow passing a different type of "socket object" instead of a real socket (e.g. some object which will forward stuff via web sockets).

Actions #13

Updated by okurz over 2 years ago

  • Due date deleted (2022-05-19)
  • Status changed from In Progress to Resolved

Correct approach, I would say :)

https://github.com/os-autoinst/os-autoinst/pull/2051 is merged and provides 100% statement coverage which I think is great and also covers both ACs so let's resolve, shall we?

Actions #14

Updated by mkittler over 2 years ago

AC1 is now fulfilled (https://app.codecov.io/gh/os-autoinst/os-autoinst/blob/master/consoles/VNC.pm is at 100 %) as well as AC2 (unit tests mock the socket).

Actions #15

Updated by AdamWill over 2 years ago

The 'update framebuffer' test added here does not work properly on big-endian (or maybe the test works properly and the code is bad? Either way, there's a problem). https://progress.opensuse.org/issues/111608

Actions

Also available in: Atom PDF