Skip to content
  • Jon Badiali's avatar
    [#8] Revisions and fixes to enable YDBTest imptp tests to run and pass · 0d7bb68b
    Jon Badiali authored
    All YottaDB APIs must be tested using the imptp portion of the YDBTest test framework. This commit includes various revisions and fixes to enable imptp tests to run and pass within YDBTest.See the YDBTest!1299 merge request for those changes.
    
    To allow correct redirection of output in `impjob.py` instances spawned during YDBTest imptp testing, it was necessary to implement a wrapper for the `ydb_stdout_stderr_adjust` Simple API function. This function is wrapped by the new `adjust_stdout_stderr` function, which was implemented in `_yottadb.c` and added to `yottadb.py`. It is not straightforward to test directly, so test coverage for this function is accomplished by the imptp tests in YDBTest.
    
    To allow YDBPython to be used and tested with address sanitization, the following changes were made:
    + Revised `setup.py` to automatically compile YDBPython with ASAN when YDB (in `$ydb_dist`) is compiled with ASAN
    + Fixed heap-buffer-overflow detected by ASAN settings by removing erroneous `+ 1` for null terminator in `memcpy()` call in `_yottadb.c`
    + Fixed heap-buffer-overflow bug detected by ASAN in `ci_wrapper()`
        - Added `gparam_list` struct to `_yottadb.h` for use in callers of `ydb_call_variadic_plist_func()`
        - Revised type of `arg_values` from `uintptr_t` to `gparam_list` in callers of `ydb_call_variadic_plist_func()`
    	+ Removed `malloc()` and `free()` calls, as `gparam_list` is now stack allocated
    	+ Updated various type casts and array assignments to match `gparam_list` member types
    + Added missing `free()` calls in various places
    
    Pipeline updates:
    + Revised CentOS jobs to use Rocky instead of CentOS to match other YDB repos
    + Broke `.test` subjob into two subjobs: `.test_ubuntu` and `.test_rocky` to allow different ASAN specifications per platform
    + Increased test verbosity in pipeline by adding `-v` to `pytest` call, allowing overview of tests passing/failing at a glance
    + Added redirect of pytest `stdout` to file and added to job artifacts to facilitate debugging
        - Added error file for `pytest` output to allow review of `pytest`'s `stderr` output
    + Revised `test_wordfreq` to output to file instead of `stdout` and added new output files to artifacts
    
    Test fixes:
    + Added `try`/`except` block to handle `KeyError` when environment variables are already unset in `test_no_ydb_gbldir()`. This resolves failures where the test tries to unset `ydb_gbldir` but it wasn't set in the environment, causing a `KeyError` exception. So, an except block catching and ignoring this exception was added to handle that case.
    + Decreased sleep time to 0.1 seconds in lock timeout tests to prevent timing-related failures. This was necessary because `lock_value` is a function that acquires a lock and sleeps for 0.2 seconds before releasing it. Therefore if the affected lines sleep for 0.2 seconds, it is more likely that by the time the `_yottadb.lock_incr()` call in each is done, the initial process has released the lock. In that case no timeout error is raised and the test fails. Reducing the timeout to 0.1 seconds (half of the time that process sleeps with the lock held) eliminates this issue by almost guaranteeing process is still sleeping while holding the lock.
        - This prevents the test from waiting so long that the lock is released before an attempt is made to acquire it.
    + Revised `test_unsigned_int_length_bytes_overflow` to use `2**32` (i.e. `UINT32_MAX + 1`) instead of `(2**32) + 1` (i.e. `UINT32_MAX + 2`) to test the off-by-one case
        - Corrected type size check in `anystr_to_buffer()` by replacing `INT32_MAX` with `UINT32_MAX`
        - Updated comments in `test_unsigned_int_length_bytes_overflow` and `anystr_to_buffer()`
    
    `README.md` updates:
    + Updated README with package instructions
    + Added cleanup instructions to README
    + Added ASAN setup to README
    + Added comments explaining lock_timeout tests sleep duration
    + Fixed tab/spaces issue and restructured instructions for clarity
    + Fixed list numbering in various places that were out of order and made one item lists unordered instead of numbered lists
    + Changed secondary headers to use `##` (`h2`) instead of `#` (`h1`) for clearer organization and consistency with HTML best practice
    
    Miscellaneous changes:
    + Added `tmp*` to `.gitignore` to ignore auto-generated `tmp.mupip` file and test subdirectories used for test database creation
    + Replaced various return values of `FALSE` with `!YDB_OK` to make return value success/failure format consistent with YDB API calls, which return `YDB_OK` on success
    + Renamed `YDB_INSTALL_DIR` to `YDB_DIST`
    + Added missing function information to docstring for `tp()`
    + Revisions to fix failure due to erroneous counting of '^' as part of global variable name during buffer population in `_yottadb.c`
        - Refactored `POPULATE_NEW_BUFFER` and `cast_pyssize_t_to_unsigned_int` into existing `anystr_to_buffer`
        - Removed a bunch of dead code and unused variables, simplified calls to `PyArg_ParseTupleAndKeywords` by reducing argument count
        - Added new `YDBPY_ERR_ARG_NOT_BYTES_LIKE` for use in generic `TypeError`s during argument parsing
    + Fixed bug in functions calling the `POPULATE_SUBS_USED_AND_SUBSARRAY()` macro wherein the value of `return_null` was not checked across macro calls, allowing failures in prior macros to go undetected. This is now fixed by the following changes:
        - Refactored `POPULATE_SUBS_USED_AND_SUBSARRAY()` into `populate_subs_used_and_subsarray()` function
        - Replaced above macro with new function and relevant error handling throughout `_yottadb.c`
        - Refactored affected functions to add explicit error checking, cleanup, and remove implicit fall-throughs via `return_null` variable
    + Corrected SIG-11 in `set()` due to unallocated default value, by adding default value and conditional `free()` in the case where the default is NOT used
    + Added default Python parameter initialization to some functions
    + Added `INT32_MAX` overflow check to `anystr_to_buffer`
    + Reduced duplication by moving buffer and subsarray population and cleanup into new `INVOKE_ANYSTR_TO_BUFFER` and `INVOKE_POPULATE_SUBS_USED_AND_SUBSARRAY_AND_CLEANUP_VARNAME` macros
    + Fixed compiler warnings on freeing of string literals by converting these literals into heap allocated variables (the compiler did not respect/understand the logic that prevented this scenario and issued this warning anyway)
    + Split `YDBTimeoutError` into `YDBLockTimeoutError` and `YDBTpTimeoutError` to distinguish timeouts issued by `lock()` and `tp()`
    + Revised `conftest.py` to store both stdout and stderr output in `execute()` function for use in debugging
    + Removed newlines before returns in `_yottadb.c`
    0d7bb68b