Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Loader test update #119

Merged
merged 2 commits into from
Sep 3, 2019
Merged

Loader test update #119

merged 2 commits into from
Sep 3, 2019

Conversation

daveh-lunarg
Copy link
Contributor

The loader_test is a unit test of loader functionality, including debug messenger. It was previously only run against the faux-runtime binaries and only on Linux, and had fallen into disrepair. This switches to using the installed runtime, if one is available, and otherwise degrades gracefully to a small number of loader-handled checks only.

This version has only been tested to date on Win10 and on a Linux system with no active runtime (skipping most tests). The faux-runtime allowed null devices, but the active runtime tests required creation of an ID3D11Device during test. Corresponding device creation code will most likely be required in the Linux paths.

Intent is to use it as an automated CI check, once complete.

The bulk of the changes are in loader_test.cpp, but some of the refactoring and bug-fixes ended up touching a relatively large number of files.

@daveh-lunarg daveh-lunarg requested a review from rpavlik August 23, 2019 21:48
@daveh-lunarg daveh-lunarg mentioned this pull request Aug 23, 2019
Closed
@daveh-lunarg
Copy link
Contributor Author

Fixed the build issue, but now have a couple test failures on Win10. Will review on Mon.

rpavlik
rpavlik reviewed Aug 26, 2019
View reviewed changes
Copy link
Contributor

@rpavlik rpavlik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to actually check out these changes, rather than review a diff, but I've got some comments in-line. Thanks for this work - there's a lot of changes here. There are some that would be great to get submitted and reviewed separately.

Are all the XR_SUCCESS == result -> XR_SUCCEEDED(result) just changed for convention's sake, or are there actual success-but-not-XR_SUCCESS result codes you were hitting?

src/api_layers/CMakeLists.txt Outdated Show resolved Hide resolved
src/common/object_info.cpp Show resolved Hide resolved
src/common/object_info.cpp Show resolved Hide resolved
src/loader/api_layer_interface.cpp Show resolved Hide resolved
src/loader/loader_core.cpp Outdated Show resolved Hide resolved
src/loader/loader_core.cpp Outdated Show resolved Hide resolved
src/loader/loader_platform.hpp Outdated Show resolved Hide resolved
src/scripts/generate_runtime_manifest.py Outdated Show resolved Hide resolved
@daveh-lunarg
Copy link
Contributor Author

I've moved two areas of discussion out into a separate PR #123, and gitlab MR #1540.

The remaining code here updates the loader test to be more functional, but isn't completely so until the above PR/MRs are included. Pushing this now as a "no harm" PR.

@daveh-lunarg daveh-lunarg merged commit 7ce26a8 into master Sep 3, 2019
@daveh-lunarg daveh-lunarg deleted the daveh-loader-tests branch September 3, 2019 18:28
@rpavlik
Copy link
Contributor

rpavlik commented Sep 3, 2019

fyi, if you can, please run ./runClangFormat.sh before committing. Makes my life easier when it comes time to sync back to GitLab :)

@daveh-lunarg
Copy link
Contributor Author

Yeah, and sorry. I use the Visual Studio clang-format built-in, but must have slipped my mind here.

rpavlik added a commit that referenced this pull request Oct 8, 2019
@rpavlik
OpenXR 1.0.3 release (7-October-2019)
f064aa4 
Patch release for the 1.0 series.

Note that this release includes changes to adjust the symbol exports from
dynamic library versions of the loader to align with the specification. Only
**core** symbols are currently exported. All extension symbols must be retrieved
using `xrGetInstanceProcAddr`.

### GitHub Pull Requests

These had been integrated into the public repo incrementally.

- General, Build, Other
  - #139 - Write output atomically at the end of generator scripts
  - #119 - Loader test updates.
  - #116 - Static analysis cleanups.
- Loader
  - #140 - Permit broader valid usage re: layers
  - #133 - Remove shwapi dependency
  - #132 - Fix directory searching for layers
  - #130 - Fix exporting of symbols on Windows.
  - #129 - Remove debug ext only when added by loader - fixes usage of debug ext
    on runtimes that do not provide it themselves.
  - #125 - Include a `OutputDebugString` logger for Win32
- Layers
  - #138 - Don't validate output enum buffer values
  - #137 - Fix incorrect filenames in the generated API layer JSON

### Internal issues

- General, Build, Other
  - Fix warnings in MSVC static code analysis mode (internal MR 1574)
  - Validation layer improvements and fixes (internal MR 1568)
  - Update vendored jsoncpp to 1.9.1 (internal MR 1523)
- Loader
  - Add ability to quiet the loader's default output (internal MR 1576)
  - Fix conformance of loader in `xrEnumerateApiLayerProperties`/`xrEnumerateInstanceExtensionProperties`
- hello_xr
  - Simplify action usage in hello_xr (internal MR 1553)
- Registry
  - Add `XR_EXT_view_configuration_depth_range` extension (internal MR 1502, internal issue 1201)
  - Reserve a Monado extension (internal MR 1541)
rhabacker pushed a commit to rhabacker/OpenXR-SDK-Source that referenced this pull request Nov 16, 2022
@rpavlik
OpenXR 1.0.3 release (7-October-2019)
2bcc4fe 
Patch release for the 1.0 series.

Note that this release includes changes to adjust the symbol exports from
dynamic library versions of the loader to align with the specification. Only
**core** symbols are currently exported. All extension symbols must be retrieved
using `xrGetInstanceProcAddr`.

### GitHub Pull Requests

These had been integrated into the public repo incrementally.

- General, Build, Other
  - KhronosGroup#139 - Write output atomically at the end of generator scripts
  - KhronosGroup#119 - Loader test updates.
  - KhronosGroup#116 - Static analysis cleanups.
- Loader
  - KhronosGroup#140 - Permit broader valid usage re: layers
  - KhronosGroup#133 - Remove shwapi dependency
  - KhronosGroup#132 - Fix directory searching for layers
  - KhronosGroup#130 - Fix exporting of symbols on Windows.
  - KhronosGroup#129 - Remove debug ext only when added by loader - fixes usage of debug ext
    on runtimes that do not provide it themselves.
  - KhronosGroup#125 - Include a `OutputDebugString` logger for Win32
- Layers
  - KhronosGroup#138 - Don't validate output enum buffer values
  - KhronosGroup#137 - Fix incorrect filenames in the generated API layer JSON

### Internal issues

- General, Build, Other
  - Fix warnings in MSVC static code analysis mode (internal MR 1574)
  - Validation layer improvements and fixes (internal MR 1568)
  - Update vendored jsoncpp to 1.9.1 (internal MR 1523)
- Loader
  - Add ability to quiet the loader's default output (internal MR 1576)
  - Fix conformance of loader in `xrEnumerateApiLayerProperties`/`xrEnumerateInstanceExtensionProperties`
- hello_xr
  - Simplify action usage in hello_xr (internal MR 1553)
- Registry
  - Add `XR_EXT_view_configuration_depth_range` extension (internal MR 1502, internal issue 1201)
  - Reserve a Monado extension (internal MR 1541)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rpavlik rpavlik rpavlik left review comments

@brycehutchings brycehutchings Awaiting requested review from brycehutchings

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@daveh-lunarg @rpavlik