Re: [PATCH v3] emacs: Make notmuch-show-next-thread return nil on failure

Subject: Re: [PATCH v3] emacs: Make notmuch-show-next-thread return nil on failure

Date: Thu, 26 May 2022 20:28:49 -0300

To: Leo Okawa Ericson, notmuch@notmuchmail.org

Cc:

From: David Bremner

Leo Okawa Ericson <git@relevant-information.com> writes:

> +test_begin_subtest "notmuch-search-show-thread returns non-nil on success"
> +test_emacs_expect_t  '(notmuch-search "id:20091117203301.GV3165@dottiness.seas.harvard.edu")
> +		      (when (notmuch-search-show-thread)
> +         		(error "Expected non-nil when successfully showing a thread"))
> +		      (when (notmuch-show-next-thread)
> +         		(error "Expected nil when there are no more threads"))
> +		      t'
> +
> +test_begin_subtest "notmuch-show-next-thread returns non-nil on success"
> +test_emacs_expect_t  '(notmuch-search "id:20091117203301.GV3165@dottiness.seas.harvard.edu")
> +		      (when (notmuch-show-next-thread)
> +         		(error "Expected non-nil when successfully showing a thread"))
> +		      (when (notmuch-show-next-thread)
> +         		(error "Expected nil when there are no more threads"))
> +		      t)'
> +

- There are some whitespace issues that git complains about (space
  before tab). That seems fixable with a judicious application of M-x
  tabify. There is also an extra space after test_emacs_expect_t.

- The tests look like there there is some over enthusiastic copy
  pasting, since both test both functions, which makes them duplicate
  tests.

- Not sure why you need to throw errors here, can't you just return t or
  nil?

- the logic of the "when" does not seem to match the string for the
  error. Are you expecting the function calls to succeed or not?

d

_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org

Thread: