Re: master 32e790f: Implement NTLM server for ntlm.el testing

[Thomas Fitzsimmons <fitzsim@fitzsim.org>]

Michael Albinus <michael.albinus@gmx.de> writes:

> fitzsim@fitzsim.org (Thomas Fitzsimmons) writes:
>
>> +Some optional tests require packages from GNU ELPA.  By default
>> +../../elpa will be checked for these packages.  If GNU ELPA is checked
>> +out somewhere else, use
>> +
>> +    make GNU_ELPA_DIRECTORY=/path/to/elpa ...
>
> When I've tried it with an up-to-date checkout of emacs master, I've got
>
> # make -C test ntlm-tests

[...]

> Ran 3 tests, 1 results as expected, 2 unexpected (2021-02-19 10:07:27+0100, 0.485233 sec)
>
> 2 unexpected results:
>    FAILED  ntlm-authentication
>    FAILED  ntlm-authentication-old-compatibility-level

Thanks for trying this.

> Not a big deal, after pulling and recompiling recent web-server ELPA
> package it was fixed. However, it makes me nervous that a checkout of
> emacs master can fail tests, w/o any reason inside the sources.

I realize this isn't ideal, but it's better than nothing, i.e., not
being able to use GNU ELPA packages as test dependencies.  And if you
expand "inside the sources" to include GNU ELPA, which I assume Emacs
maintainers who have an elpa checkout do, then I think it's OK (maybe
even desirable?) to have to look into GNU ELPA-dependent test failures.

That said, if there is no elpa.git checkout at ../elpa, or if
GNU_ELPA_DIRECTORY=/nonexistent, then the tests are just skipped.

(Maybe as a result of the bundling discussions, GNU ELPA will become a
submodule of emacs.git which would make the dependency more reliable.)

> Could you pls provide more advanced checks for dependencies? For
> example, instead of
>
>  (and (featurep 'url-http-ntlm) (featurep 'web-server))
>
> you could check a proper version of the packages?

I'd rather not add the complexity, e.g., depending on package.el.
Beyond this first requirement to update web-server to the latest, I
don't see how a version check would be useful.  I tried to implement a
specific check for the ws-parse functionality, but I couldn't find an
easy way of confirming the new NTLM path.

> Furthermore, when the ELPA packages are not present, we see in the make
> test output:
>
> Warning (emacs): Cannot find one or more GNU ELPA packages
> Warning (emacs): Skipping NTLM authentication tests
> Warning (emacs): See GNU_ELPA_DIRECTORY in test/README
> Running 3 tests (2021-02-19 10:26:00+0100, selector `(not (tag :unstable))')
>   skipped  1/3  ntlm-authentication (0.000133 sec)
>   skipped  2/3  ntlm-authentication-old-compatibility-level (0.000097 sec)
>    passed  3/3  ntlm-time-to-timestamp (0.000136 sec)
>
> Is it necessary to be such chatty? The tests are skipped (like other
> tests), fine.

I debated not putting in those warnings.  I wanted to point out why the
tests are being skipped, and in particular that it's easy to have them
not be skipped if you have a GNU ELPA checkout (versus tests that are
unavoidably skipped because they depend on a different OS or
architecture).

Basically, I'd like to encourage Emacs maintainers to not skip these
tests; the reason I wrote them was to prevent regressions like Bug#43566
(merged with Bug#44195 and Bug#44439), which broke all uses of NTLM in
an Emacs point release.

Thomas