On 8/5/20 3:39 AM, Arkadiusz Hiler wrote:
On Tue, Aug 04, 2020 at 10:25:48AM -0500, Zebediah Figura wrote:
+static void test_hardlink(void) +{
- DWORD rc;
- HANDLE hfile;
- BY_HANDLE_FILE_INFORMATION info;
- hfile = CreateFileA("file", GENERIC_WRITE, 0, NULL, CREATE_ALWAYS,
FILE_ATTRIBUTE_NORMAL, NULL);- ok(hfile != INVALID_HANDLE_VALUE, "failed to create a file\n");
- if (hfile == INVALID_HANDLE_VALUE)
- {
skip("skipping hardlink tests\n");return;- }
I'm personally of the opinion that these kinds of checks aren't particularly useful—if a test fails, that's already a problem and needs to be addressed.
<SNIP> >> +START_TEST(fsutil) >> +{ >> + char tmpdir[MAX_PATH]; >> + >> + GetTempPathA(MAX_PATH, tmpdir); >> + SetCurrentDirectoryA(tmpdir); >> + trace("%s\n", tmpdir); > > Is this trace() message doing anything particularly useful?
Those two were borrowed from other tests. I was checking how xcopy and find are tested while looking for inspiration.
One can argue that if we haven't managed to create the source file everything past that is just noise, but I also see the point of making such failures more colorful and noticable :-)
As of that trace - I thought it may be useful for debugging to see where the files are located for manual inspection upon failure.
Anyway, I went with all of your suggestion, as I am still developing my own taste here.
Sure, and at least the first comment is arguably just my opinion.
If you want to be more verbose about failure to create a file, I'd put that information in the ok() message. Failing that, I'd at least guard the trace() with "if (winetest_debug > 1)".
For some context, we also have problems with tests printing too much information (we have a hard limit of 15 MB per test result hosted on test.winehq.org, and recently we've been running rather close to that limit.)