Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210928221111.1162779-1-dlatypov@google.com> <CABVgOS=0K78N+KMK3km5TKVDD9L8AMRpNCfvihCqU2h3U-oE-w@mail.gmail.com>
In-Reply-To: <CABVgOS=0K78N+KMK3km5TKVDD9L8AMRpNCfvihCqU2h3U-oE-w@mail.gmail.com>
From:   Daniel Latypov <dlatypov@google.com>
Date:   Tue, 28 Sep 2021 19:12:25 -0700
Message-ID: <CAGS_qxpkak7QEfPvQhozuJt5VN=9+hcNOjrEryVj=hV8A1mcDQ@mail.gmail.com>
Subject: Re: [PATCH] kunit: tool: misc fixes (unused vars, imports, leaked files)
To:     David Gow <davidgow@google.com>
Cc:     Brendan Higgins <brendanhiggins@google.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        KUnit Development <kunit-dev@googlegroups.com>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>,
        Shuah Khan <skhan@linuxfoundation.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Sep 28, 2021 at 5:34 PM 'David Gow' via KUnit Development
<kunit-dev@googlegroups.com> wrote:
>
> On Wed, Sep 29, 2021 at 6:11 AM Daniel Latypov <dlatypov@google.com> wrote:
> >
> > Drop some variables in unit tests that were unused and/or add assertions
> > based on them.
> >
> > For ExitStack, it was imported, but the `es` variable wasn't used so it
> > didn't do anything, and we were leaking the file objects.
> > Refactor it to just use nested `with` statements to properly close them.
> >
> > And drop the direct use of .close() on file objects in the kunit tool
> > unit test, as these can be leaked if test assertions fail.
>
> To clarify for a python novice: this is referring to using "with" so
> that the file isn't leaked if the assertion fails, rather than
> suggesting that leaks are okay for failing tests, right?

Correct.
Ah, I see how it can be misread now.

But on that note, it's probably fine to leak the files as CPython
should close the file object during GC.
And tests failing should make those file objects go out of scope immediately.

>
> > Signed-off-by: Daniel Latypov <dlatypov@google.com>
> > ---
>
> These all seem sensible to me. Thanks for cleaning this up!
>
> Reviewed-by: David Gow <davidgow@google.com>
>
> -- David
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/CABVgOS%3D0K78N%2BKMK3km5TKVDD9L8AMRpNCfvihCqU2h3U-oE-w%40mail.gmail.com.