[Bug 54656] New: The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

Bug ID: 54656
           Summary: The winscard tests are not run on the GitLab CI
           Product: Wine
           Version: unspecified
          Hardware: x86-64
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: winscard
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

The winscard tests are not run on the GitLab CI:

winscard=dll is missing

See the gitlab-debian-* Linux results on:
http://winetest.dolphin/data/tests/winscard:winscard.html

I'm putting this as a Wine bug rather than a Wine GitLab one because I think
the fix is to update the tools/gitlab/image.docker file.

Also I'd argue this is a regression introduced by the commit that added the
dependency on libpcsclite because the Docker image update should have been made
there.

commit 8490c43f38e306fef7b5fed3ffcb256efd73af58
Author:     Hans Leidekker 
AuthorDate: Thu Feb 16 09:56:12 2023 +0100

winscard: Implement SCardEstablish/ReleaseContext() on top of libpcsclite.

[Bug 54656] The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

François Gouget  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
    Regression SHA1|                            |8490c43f38e306fef7b5fed3ffc
                   |                            |b256efd73af58
           Keywords|                            |regression, source,
                   |                            |testcase

[Bug 54656] The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

--- Comment #1 from François Gouget  ---
I sent a merge request for this which may or may not works.

As far as I can tell it did not trigger an update of the build environment so I
expect the winscard tests to still be skipped. But if the build environment
were to be updated using the new image.docker file I believe it would end up
running the tests. I don't know if there's special magic to be done to cause an
MR to update the build environment.

https://gitlab.winehq.org/wine/wine/-/merge_requests/2362

[Bug 54656] The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

--- Comment #2 from Hans Leidekker  ---
(In reply to François Gouget from comment #0)
> The winscard tests are not run on the GitLab CI:
> winscard=dll is missing
>
> See the gitlab-debian-* Linux results on:
> http://winetest.dolphin/data/tests/winscard:winscard.html
>
> I'm putting this as a Wine bug rather than a Wine GitLab one because I think
> the fix is to update the tools/gitlab/image.docker file.
>
> Also I'd argue this is a regression introduced by the commit that added the
> dependency on libpcsclite because the Docker image update should have been
> made there.

I don't see why this is a regression. There were no tests before that commit.

[Bug 54656] The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

--- Comment #3 from François Gouget  ---
Before the GitLab CI had all the libraries needed to run all the Wine tests.
Now it does not. So it's a regression.

[Bug 54656] The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

--- Comment #4 from François Gouget  ---
More infrastructure may be needed to actually get meaningful results from the
tests: if one just installs the libraries the tests immediately get an
SCARD_E_NO_SERVICE error and bail out (which makes sense):

can't establish context, make sure pcscd is running

[Bug 54656] The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

--- Comment #5 from Alexandre Julliard  ---
(In reply to François Gouget from comment #3)
> Before the GitLab CI had all the libraries needed to run all the Wine tests.
> Now it does not. So it's a regression.

By this criteria pretty much everything would be a regression, that's not a
useful definition.

[Bug 54656] The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

--- Comment #6 from Hans Leidekker  ---
(In reply to François Gouget from comment #4)
> More infrastructure may be needed to actually get meaningful results from
> the tests: if one just installs the libraries the tests immediately get an
> SCARD_E_NO_SERVICE error and bail out (which makes sense):
>
> can't establish context, make sure pcscd is running

Right, to run all tests we'd need pcscd running and a smart card attached.

[Bug 54656] The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

--- Comment #7 from François Gouget  ---
(In reply to Alexandre Julliard from comment #5)
> (In reply to François Gouget from comment #3)
> > Before the GitLab CI had all the libraries needed to run all the Wine tests.
> > Now it does not. So it's a regression.
>
> By this criteria pretty much everything would be a regression, that's not a
> useful definition.

By that definition >99% of the commits are not regressions since they don't
introduce new Wine dependencies. And I'd argue that now that updating the
GitLab CI falls on the shoulders of Wine developers, a commit that fails to
update the GitLab CI to test the new code is a regression.

[Bug 54656] The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

--- Comment #8 from Alexandre Julliard  ---
(In reply to François Gouget from comment #7)
> By that definition >99% of the commits are not regressions since they don't
> introduce new Wine dependencies. And I'd argue that now that updating the
> GitLab CI falls on the shoulders of Wine developers, a commit that fails to
> update the GitLab CI to test the new code is a regression.

The definition is not limited to dependencies, but yes, >99% of the commits are
not regressions, that's the way it should be. That's what makes things like the
regression hall of shame useful. If we apply the keyword to any change of
behavior that someone doesn't like, it becomes useless.

A regression has to be something that can be pointed to a specific piece of
code that worked, and got broken by a change, and where the fix will most
likely involve refining the change.

For instance, a bug in a newly-added dll would not be considered a regression.
From the user's point of view it may be a new problem, since obviously the bug
didn't happen when the dll didn't exist, but from the developer's point of
view, there's no previous version of the code that worked, so it hasn't
regressed. The goal of Bugzilla is to provide meaningful information to
developers, and there's no useful information to gather from the fact that a
bug didn't happen when the code didn't exist.

So not running a test that didn't exist previously, to test code that didn't
exist previously either, should not be considered a regression in the sense of
the Bugzilla regression keyword.

[Bug 54656] The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

--- Comment #9 from François Gouget  ---
Ok. So merge requests that add a Wine dependency do not have to update
tools/gitlab to ensure the GitLab CI has the required libraries for the build,
so long as the affected code can build without them. Also there is no
requirement to update the GitLab CI to ensure the corresponding tests can be
run.

Is this policy documented somewhere?
(I don't see anything on https://wiki.winehq.org/Submitting_Patches)

So who's job is it to update the GitLab CI? The GitLab CI maintainer
presumably?

Does that mean that bugs about missing dependencies should be filed against the
"WineHQ Gitlab" product rather than against Wine? (even though it seems to be
the fix would be in Wine)

[Bug 54656] The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

François Gouget  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|regression                  |
    Regression SHA1|8490c43f38e306fef7b5fed3ffc |
                   |b256efd73af58               |

[Bug 54656] The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

--- Comment #10 from Alexandre Julliard  ---
(In reply to François Gouget from comment #9)
> So who's job is it to update the GitLab CI? The GitLab CI maintainer
> presumably?

Ultimately, yes, but sending a merge request is a good way to get it on my todo
list.

The way it's configured, the image is re-generated only once the merge request
is merged, because we don't want to have a separate image for every fork. And
because we use the same image for everything I have to check it by hand first.
So it's actually better to make it a separate MR instead of it being part of
the MR that adds the dependency.

[Bug 54656] The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

--- Comment #11 from François Gouget  ---
I added an "Introducing new dependencies" section to the Submitting Patches
page to codify this policy. Review / adjust as appropriate.

[Bug 54656] The winscard tests are not run on the GitLab CI

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54656

--- Comment #12 from Hans Leidekker  ---
The docker image contains libpcsclite-dev now (64-bit only) and winscard tests
are run on GitLab CI. Can we close this bug?