[master] add show diff for new file in file.managed

[nicholasmhughes]

What does this PR do?

See issue for details.

What issues does this PR fix or reference?

Fixes: #65546

Previous Behavior

State output only showed newfile: <name> or diff: New file in changes.

New Behavior

Diff is now able to be optionally shown.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

• Docs
• Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
• Tests written/updated

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

[nicholasmhughes]

@twangboy do you have any ideas why these Windows tests might be failing? Not sure if there's anything interesting about how that OS handles temp files.

[nicholasmhughes]

@twangboy do you have any ideas why these Windows tests might be failing? Not sure if there's anything interesting about how that OS handles temp files.

@twangboy , any thoughts on this? seems to work fine for all other operating systems

Also, can someone add an Argon v3008.0 label to this? Thanks!

cc: @dwoz

[twangboy]

I think it's trying to open that file that doesn't exist yet it's throwing that error. Maybe the lstat command or something. I don't think it's actually a permissions issue. I think that's just a misleading error message. There are also some issues with line endings... this should get you close.

 def test_file_managed_new_file_diff(file, tmp_path):
+    tmp_path.mkdir(exist_ok=True, parents=True)
     name = tmp_path / "new_file_diff.txt"
+    ls = os.linesep
+    expected = {"diff": f"--- \n+++ \n@@ -0,0 +1 @@\n+EITR{ls}"}
     ret = file.managed(str(name), contents="EITR", new_file_diff=True, test=True)
-    assert ret.changes == {"diff": "--- \n+++ \n@@ -0,0 +1 @@\n+EITR\n"}
+    assert ret.changes == expected
     assert not name.exists()
     ret = file.managed(str(name), contents="EITR", new_file_diff=True)
-    assert ret.changes == {"diff": "--- \n+++ \n@@ -0,0 +1 @@\n+EITR\n"}
+    assert ret.changes == expected
     assert name.exists()

I'm still looking at it, but I'm getting pulled to other things.

This also needs a rebase and conflicts resolved

[twangboy]

@nicholasmhughes OK, I got the tests working on Windows. I had to change the way you were opening the temp file. For some reason the tempfile.NamedTemporaryFile was keeping the lock on the file so Windows couldn't open it to write its contents. I saw above where they had used a salt util to create the temporary file, so I followed that paradigm. That seemed to take care of the permission denied error.

In the test on Windows it seems to return an additional dict entry in changes for "newfile". Maybe it doesn't do that on Linux. So, all the tests will now fail on Linux. I'll let you decide if you wanna hunt that down or modify the test for Linux.

[twangboy]

Also, this still needs a rebase and conflict resolution

[twangboy]

Looks like fixing the test for Windows breaks it for everything else. I don't know why you get a "new_file" on Windows but not the other OS's. It seems to me like that would be valid output if a new file is being created.