Null tests.142

[kiihne-noaa]

Describe your changes

Added tests in fre/make/null_example for checkout

Issue ticket number and link (if applicable)

creating this so Dana can help with debugging of issues with paths

Checklist before requesting a review

@singhd789

[ilaflott]

for the rm -rf, instead of subprocess, consider shutil.rmtree

[ilaflott]

@singhd789 @thomas-robinson perhaps create_checkout_script should return the full path to checkout.sh? instead of just printing it to screen?

[singhd789]

Well, the whole path to the checkout script is printed (and it's made executable) - that way the user knows where it is and could also run it themselves if they want. What would be another benefit of returning it as well? Just curious

[ilaflott]

this test + the otheres could become a simple:

compile_sh_path =  create_checkout_script (<BLAH ARGS>)
assert Path(compile_sh_path).exists()

similarly, elsewhere in the code base, if create_checkout_script is being used, that returned string object could be actually leveraged by other functions. If it's just print, then there is no option to do so

[ilaflott]

OK so i saw that what brought this PR's checks from red-x to green-check is the change of this functions argument from a boolean value to an integer 2. This suggests to me that it's hard to track what these arguments are supposed to be when developing the code, so checkout_create needs explicit keyword arguments, not positional.

this line should read like:

create_checkout_script.checkout_create(
    yamlfile = YAMLFILE, 
    platform = PLATFORM, 
    target = TARGET, 
    no_parallel_checkout = False, 
    jobs = 2, 
    execute = True, 
    verbose = False )

[singhd789]

If you use the ci.gnu platform with the TEST_BUILD_DIR defined for the modelRoot (

fre-cli/fre/make/tests/null_example/platforms.yaml

Line 20 in a9730e0

modelRoot: ${TEST_BUILD_DIR}/fremake_canopy/test

), I don't think you'll even need CWD

[singhd789]

I think jobs has a default value here (Default value set in fremake.py click options for jobs - when you don't explicitly define it, 4 prints out). So it might always be "True" or defined.

[kiihne-noaa]

@singhd789 @ilaflott This is now ready for final review

[singhd789]

Nice changes!! I think besides moving your test_checkout_null_yaml.py up one directory (rename test_create_checkout.py) and addressing the if jobs == 'False' and execute: error check (I think we settled on checking if jobs was a boolean and what not instead), this is pretty good.

[singhd789]

Ok sorry, just noticed. I recommend changing OUT to OUT = f"{TEST_DIR}/checkout_out", or something checkout related so we know it's associated with this test. It might be checking for output files in the wrong directory here (that path was from the run-fremake test).

[singhd789]

Instead of subprocess.run, let's use shutil.rmtree(OUT) (for directory removal) of Path(.......).unlink (I think it's this) (for file removal) here.

[singhd789]

I think we'll need to check that actual contents of the file here and add an assert here as well (for file creation).

If the no_parallel option is defined, the checkout script is going to be different. Maybe we can add a pytest skip here and work on updating this test in the next PR.