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

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

[Michael Albinus]

fitzsim@fitzsim.org (Thomas Fitzsimmons) writes:

Hi Thomas,

> +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

--8<---------------cut here---------------start------------->8---
# make -C test ntlm-tests
make: Entering directory '/usr/local/src/emacs/test'
make[1]: Entering directory '/usr/local/src/emacs/test'
  GEN      lisp/net/ntlm-tests.log
Running 3 tests (2021-02-19 10:07:26+0100, selector `(not (tag :unstable))')
Contacting host: localhost:8080
Test ntlm-authentication backtrace:
  signal(ert-test-failed (((should (equal (ntlm-tests--start-server-au
  ert-fail(((should (equal (ntlm-tests--start-server-authenticate-stop
  (if (unwind-protect (setq value-14 (apply fn-12 args-13)) (setq form
  (let (form-description-16) (if (unwind-protect (setq value-14 (apply
  (let ((value-14 'ert-form-evaluation-aborted-15)) (let (form-descrip
  (let* ((fn-12 #'equal) (args-13 (condition-case err (let ((signal-ho
  (closure (t) nil (let ((value-10 (gensym "ert-form-evaluation-aborte
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name ntlm-authentication :documentation "C
  ert-run-or-rerun-test(#s(ert--stats :selector (not (tag :unstable))
  ert-run-tests((not (tag :unstable)) #f(compiled-function (event-type
  ert-run-tests-batch((not (tag :unstable)))
  ert-run-tests-batch-and-exit((not (tag :unstable)))
  eval((ert-run-tests-batch-and-exit '(not (tag :unstable))) t)
  command-line-1(("-L" ":." "-L" "./../../elpa/packages/url-http-ntlm/
  command-line()
  normal-top-level()
Test ntlm-authentication condition:
    (ert-test-failed
     ((should
       (equal
	(ntlm-tests--start-server-authenticate-stop-server)
	ntlm-tests--successful-result))
      :form
      (equal "HTTP/1.1 500 Internal Server Error\12Content-type: text/plain\12\12un-support protocol: :NTLM" "HTTP/1.1 200 OK\12\12Authenticated.\15\12")
      :value nil :explanation
      (arrays-of-different-length 87 33 "HTTP/1.1 500 Internal Server Error\12Content-type: text/plain\12\12un-support protocol: :NTLM" "HTTP/1.1 200 OK\12\12Authenticated.\15\12" first-mismatch-at 9)))
   FAILED  1/3  ntlm-authentication (0.124960 sec)
Contacting host: localhost:8080
Test ntlm-authentication-old-compatibility-level backtrace:
  signal(ert-test-failed (((should (equal (ntlm-tests--start-server-au
  ert-fail(((should (equal (ntlm-tests--start-server-authenticate-stop
  (if (unwind-protect (setq value-21 (apply fn-19 args-20)) (setq form
  (let (form-description-23) (if (unwind-protect (setq value-21 (apply
  (let ((value-21 'ert-form-evaluation-aborted-22)) (let (form-descrip
  (let* ((fn-19 #'equal) (args-20 (condition-case err (let ((signal-ho
  (closure (t) nil (let ((value-17 (gensym "ert-form-evaluation-aborte
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name ntlm-authentication-old-compatibility
  ert-run-or-rerun-test(#s(ert--stats :selector (not ...) :tests [...
  ert-run-tests((not (tag :unstable)) #f(compiled-function (event-type
  ert-run-tests-batch((not (tag :unstable)))
  ert-run-tests-batch-and-exit((not (tag :unstable)))
  eval((ert-run-tests-batch-and-exit '(not (tag :unstable))) t)
  command-line-1(("-L" ":." "-L" "./../../elpa/packages/url-http-ntlm/
  command-line()
  normal-top-level()
Test ntlm-authentication-old-compatibility-level condition:
    (ert-test-failed
     ((should
       (equal
	(ntlm-tests--start-server-authenticate-stop-server)
	ntlm-tests--successful-result))
      :form
      (equal "HTTP/1.1 500 Internal Server Error\12Content-type: text/plain\12\12un-support protocol: :NTLM" "HTTP/1.1 200 OK\12\12Authenticated.\15\12")
      :value nil :explanation
      (arrays-of-different-length 87 33 "HTTP/1.1 500 Internal Server Error\12Content-type: text/plain\12\12un-support protocol: :NTLM" "HTTP/1.1 200 OK\12\12Authenticated.\15\12" first-mismatch-at 9)))
   FAILED  2/3  ntlm-authentication-old-compatibility-level (0.102966 sec)
   passed  3/3  ntlm-time-to-timestamp (0.000149 sec)

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
--8<---------------cut here---------------end--------------->8---

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.

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?

Furthermore, when the ELPA packages are not present, we see in the make
test output:

--8<---------------cut here---------------start------------->8---
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)
--8<---------------cut here---------------end--------------->8---

Is it necessary to be such chatty? The tests are skipped (like other
tests), fine.

Best regards, Michael.

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

[Thomas Fitzsimmons]

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

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

[Michael Albinus]

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

Hi Thomas,

>> 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.

Of course I appreciate to use GNU ELPA packages for tests. But you
cannot assume that everybody has an ELPA git checkout. Given, that the
packages live in their own branches or repositories.

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

There are just three Emacs maintainers, and they might run an ELPA
checkout. But ordinary Emacs developers might live w/o an ELPA checkout,
and use just packages provided by the default mechanism, and selected by
them. We shall be prepared also for this case (and maybe extend the way
how GNU ELPA packages are accessed when running the Emacs tests).

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

Maybe. But we aren't at this point yet. And since packages have their
own repository life, I doubt that "git pull" from the Emacs repository
will bring us also fresh packages checkout.

>> 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.

That's just *this* case. Other changes might apply to your packages in
the future, which require newer package code. And I'm thinking also
about a more general approach for other Emacs tests, how to use GNU ELPA
packages as dependency.

And you don't need package.el. Require lisp-mnt, then you could do
something like

--8<---------------cut here---------------start------------->8---
(defvar ntlm-tests--dependencies-present
  (and (featurep 'url-http-ntlm)
       (version<= "2.0.4" (lm-version (locate-file "url-http-ntlm.el" load-path)))
       (featurep 'web-server)
       (version<= "0.1.2" (lm-version (locate-file "web-server.el" load-path))))

  "Non-nil if GNU ELPA test dependencies were loaded.")
--8<---------------cut here---------------end--------------->8---

>> 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.

Again, not every Emacs developer has an ELPA checkout. And we shouldn't
be bossy to them.

> Thomas

Best regards, Michael.

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

[Thomas Fitzsimmons]

Hi Michael,

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

> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

[...]

>>> 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.
>
> That's just *this* case. Other changes might apply to your packages in
> the future, which require newer package code. And I'm thinking also
> about a more general approach for other Emacs tests, how to use GNU ELPA
> packages as dependency.

OK, makes sense.

> And you don't need package.el. Require lisp-mnt, then you could do
> something like

[...]

OK, thanks.  I've pushed a variation on your suggestion that checks for
the NTLM ws-parse functionality (web-server's GNU ELPA version has not
been bumped yet).

>>> 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.
>
> Again, not every Emacs developer has an ELPA checkout. And we shouldn't
> be bossy to them.

Sure, I removed the warnings.

Thomas

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

[Michael Albinus]

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> Hi Michael,

Hi Thomas,

[...]

> OK, thanks.  I've pushed a variation on your suggestion that checks for
> the NTLM ws-parse functionality (web-server's GNU ELPA version has not
> been bumped yet).

[...]

> Sure, I removed the warnings.

Thanks!

> Thomas

Best regards, Michael.

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

[Stephen Leake]

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

> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

>> (Maybe as a result of the bundling discussions, GNU ELPA will become a
>> submodule of emacs.git which would make the dependency more reliable.)
>
> Maybe. But we aren't at this point yet. And since packages have their
> own repository life, I doubt that "git pull" from the Emacs repository
> will bring us also fresh packages checkout.

"Bundled ELPA packages" will have a release branch in emacs.git, which
will be checked out into emacs/elpa if --recurse-submodules is specified
with 'clone'.

But only packages that are likely to be used by many users will be
bundled; I'm guessing url-http-ntlm.el and web-server.el will not be.

-- 
-- Stephe