adds unit/integration tests

[dpfens]

To address #63, adds integration tests for remaining functions in c_api.rs to c_api.rs file, and adds test for WasmModule::load_from_file.

[vmwclabot]

@dpfens, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

[vmwclabot]

@dpfens, we have received your signed contributor license agreement. The review is usually completed within a week, but may take longer under certain circumstances. Another comment will be added to the pull request to notify you when the merge can proceed.

[vmwclabot]

@dpfens, VMware has approved your signed contributor license agreement.

[ereslibre]

Thank you for this contribution @dpfens! LGTM, some minor changes suggested :)

[dpfens]

@ereslibre I applied the changes you suggested, in addition to splitting out the module.rs test for load_to_file into 2 separate tests, one for testing success, and one for testing failure.

[gzurl]

Thanks @dpfens for your contribution!

[gzurl]

Just to keep coherence with other test cases naming convention:

Suggested change

	fn wasm_wasm_module_load() {
	fn wasm_module_load_existing_nonexisting() {

[gzurl]

@dpfens everything looks great. We just need to choose a naming convention (I pointed out to some examples).

I guess there are three questions:

• Should unit test functions in Rust need to start with test_ prefix? I'd say no, but open to discuss.
• For a unit test, we are using the same method name suffixed by a hint of the specific test (ie: _null_or_empy, _utf8_encoding_errors, etc. Do we want to keep it this way? I'd like to keep it since its self explanatory and nice to read when running cargo test.
• How should we suffix the general use case? We were using _general so far, but @dpfens added _success and I like it more. So, I'd advocate for that.

@ereslibre @Angelmmiguel @assambar any opinion?

[ereslibre]

Should unit test functions in Rust need to start with test_ prefix? I'd say no, but open to discuss.

No, I think the test should be named in the best way it describes what it tests, and I'd say a mandatory test_ prefix is not adding a lot of value in general.

For a unit test, we are using the same method name suffixed by a hint of the specific test (ie: _null_or_empy, _utf8_encoding_errors, etc. Do we want to keep it this way? I'd like to keep it since its self explanatory and nice to read when running cargo test.

I think it's good to have some structure. I like to have descriptive test names, but I would also not force for all tests to follow the same convention, only those that are very related to each other. In general I think it's fine to be descriptive to what the test is doing, and also meaningful with the code under test, so although structure is good sometimes, I would not enforce this.

In my opinion unit tests should be very straightforward and just with a quick look at its body it should be pretty obvious what it's doing.

How should we suffix the general use case? We were using _general so far, but @dpfens added _success and I like it more. So, I'd advocate for that.

I am fine with _success, but as I said I am not very fond of having strict rules when it comes to unit test naming other than it should be descriptive of what it does.

[gzurl]

Merged!
Thanks @dpfens