Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-15672: add temp directory removal for tests called and used tempfile. #15675

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

wendycwong
Copy link
Contributor

@wendycwong wendycwong commented Aug 3, 2023

This PR removed all temp files used in python unit test for this issue: #15672

@wendycwong wendycwong force-pushed the wendy_GH-15672_remove_temp_directories_rel branch 3 times, most recently from f396598 to 4dfc8b2 Compare August 8, 2023 18:55
Copy link
Contributor

@tomasfryda tomasfryda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably prefer to modify the test runner a bit instead of making so many changes that won't help in future tests.

I would suggest something like:

def standalone_test(test, init_options={}):
    if not h2o.connection() or not h2o.connection().connected:
        print("Creating connection for test %s" % test.__name__)
        h2o.init(strict_version_check=False, **init_options)
        print("New session: %s" % h2o.connection().session_id)

    h2o.remove_all()

    h2o.log_and_echo("------------------------------------------------------------")
    h2o.log_and_echo("")
    h2o.log_and_echo("STARTING TEST "+test.__name__)
    h2o.log_and_echo("")
    h2o.log_and_echo("------------------------------------------------------------")
    import os
    import tempfile
    with tempfile.TemporaryDirectory() as dir:  # Create a temporary dir that will clean itself
        tempfile._once_lock.acquire()  # Better lock to avoid race conditions (but since we don't test in multiple threads (to avoid side-effects on the backend) it should not matter)
        old_tempdir = tempfile.tempdir
        try:
            tempfile.tempdir = dir  # Tell tempfile to use this temporary dir as the root for all other temporary dirs
            test()
        finally:
            tempfile.tempdir = old_tempdir
            tempfile._once_lock.release()

To test it, you can tell python what should be the default root tempdir by setting TMPDIR environment variable. (But you have to do it before tempdir module is initialized as it reads the value only once).

$ TMPDIR=/my/favorite/dir/ python pyunit_...

NOTE: This should work in python 3.2 and above.

…Fryda suggestion. It is also destroyed after each test.
@wendycwong wendycwong force-pushed the wendy_GH-15672_remove_temp_directories_rel branch from b799ccf to c702d86 Compare August 15, 2023 16:40
@mn-mikke mn-mikke modified the milestones: 3.42.0.4, 3.42.0.3 Aug 16, 2023
@wendycwong wendycwong merged commit 3e1c5cc into rel-3.42.0 Aug 16, 2023
2 checks passed
@wendycwong wendycwong deleted the wendy_GH-15672_remove_temp_directories_rel branch August 16, 2023 16:07
Mohit1345 pushed a commit to Mohit1345/h2o-3 that referenced this pull request Aug 17, 2023
…the change should be made in standalone test. He provides the code and I just copied and pasted. (h2oai#15675)
mn-mikke added a commit that referenced this pull request Oct 10, 2023
…_status() in h20.py #7079 [nocheck] (#15653)

* [GH-15687] Extract string comparators from Duke library (#15692)

* [GH-15680] Adding installation disclaimer in documentation (#15681)

* ht/adding disclaimer in docs

* ht/added flow & download page

* ht/updated download page

* ht/added security documentation section

* ht/user guide updates

* ht/uniformity

* ht/added additional note to welcome page

* callouts

* ht/banner color switch

* ht/rename callout

---------

Co-authored-by: Jeff Fohl <[email protected]>

* GH-15672: Tomas Fryda suggested instead of changing every test, the change should be made in standalone test.  He provides the code and I just copied and pasted. (#15675)

* Added verbose paramerter to init() in h2o.py also guarded h2oconn.cluster.show_status()

* changes verbose param position before ** kwargs

* final commit

* removed space, and made it in rel-3.42.0

* added space

---------

Co-authored-by: Marek Novotný <[email protected]>
Co-authored-by: Hannah <[email protected]>
Co-authored-by: Jeff Fohl <[email protected]>
Co-authored-by: wendycwong <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants