Tony Wasserka wrote:
This is my first test, so I think it's better to ask for feedback on wine-devel, first.
In all of your patch:
- if(!obj) return 0;
Please add space after "if" to make it consistent and more readable: if (!obj) return 0;
hr=IDirect3DDevice9_CreateTexture
Please add spaces to the both sides of "=".
+#define CHECK_REF(obj, exp) \
ok(exp==get_ref((IUnknown*)(obj)), "Invalid refcount. Expected %d, got %d\n", exp, get_ref((IUnknown*)(obj)));
You should make it an inine perhaps to not call get_ref twice on the same object.
+#define CHECK_RELEASE(obj, exp) \
- { \
if(obj) { \int ref=IUnknown_Release((IUnknown*)(obj)); \ok(ref==exp, "Invalid refcount. Expected %d, got %d\n", exp, ref); \} else skip("Object was NULL before Release\n"); \- }
This also should be a function. And please drop skip - it should only be used to skip parts of the test. Here you just skipping one check which should always be valid.
+#define CHECK_CALL(call, name, exp) \
- hr=(call); \
- ok(hr==(exp), "%s returned %#x, expected %#x\n", (name), hr, (exp));
Don't use macros like this. Put it all in the code. It's bad thing to have macro modify some variables (hr) you don't explicitly specify as params or return.
- ID3DXSprite *sprite=NULL;
- IDirect3DDevice9 *cmpdev=NULL;
- IDirect3DTexture9 *tex1=NULL, *tex2=NULL;
No need to initialize variables that you always assign to.
- if(FAILED(hr=IDirect3DDevice9_CreateTexture(device, 64, 64, 1, 0, D3DFMT_A8R8G8B8, D3DPOOL_DEFAULT, &tex1, NULL))) {
skip("Couldn't create first texture, CreateTexture returned %#x\n", hr);return;- }
Failure to create a texture should be considered a test - use ok(). And please put hr=IDirect3DDevice9_CreateTexture() on the separate line.
- if(SUCCEEDED(hr)) {
D3DXMATRIX identity;D3DXMatrixIdentity(&identity);check_mat(mat, identity);- } else skip("No matrix to test\n");
No need to use skip() here.
Vitaliy.