-
Notifications
You must be signed in to change notification settings - Fork 129
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.
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
Create more tests of NeXus::File
using based on old napi
tests
#39027
Conversation
This one looks like a genuine test failure so I will leave it alone. |
Framework/Nexus/test/NeXusFileTest.h
Outdated
|
||
// cleanup and return | ||
fileid.close(); | ||
cout << "all ok - done\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know that the "all ok" messages are still needed since this is asserting things are equal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was really helpful for debugging, especially with segfaults/stack overflows, to know that a test had definitively ended. But I can make them more descript.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copying the tests over to NexusFile is a good step. I look forward to the c-api tests going away soon.
…39027) * save progress * more output to see source of error * change leak test 3 on windows * error macro in napi_test for better readability * link test working * slab tests * try fixing the windows test failure * fix windows test issue for real * fix test that windows is incapable of running * last time * maybe the problem is file names? * address comments remove test data dependency * maybe it was napi_test.cpp all along * try using correct macro * try again * the problem was literally the first thing I tried
…`ornl-next` (#39060) Create more tests of `NeXus::File` using based on old `napi` tests (#39027) * save progress * more output to see source of error * change leak test 3 on windows * error macro in napi_test for better readability * link test working * slab tests * try fixing the windows test failure * fix windows test issue for real * fix test that windows is incapable of running * last time * maybe the problem is file names? * address comments remove test data dependency * maybe it was napi_test.cpp all along * try using correct macro * try again * the problem was literally the first thing I tried
Description of work
Summary of work
This adds more tests or
NeXus::File
, which will be very important before we begin any work on removingnapi
. These tests are based on tests ofnapi
, so should ensure continuity of behavior.Related to #38332
Follow-on to PR #39010, adding tests taken from
napi_test.cpp
, originally written fornapi
.Further detail of work
Writing this I had to compare to the
napi_test.cpp
file, and I got sick of the incessantcluttering it. So I introduced a macro to make these all one line. It should not change any behavior.
To test:
This is adding tests, and not messing with any functionality. Make sure the new tests pass.
Reviewer
Please comment on the points listed below (full description).
Your comments will be used as part of the gatekeeper process, so please comment clearly on what you have checked during your review. If changes are made to the PR during the review process then your final comment will be the most important for gatekeepers. In this comment you should make it clear why any earlier review is still valid, or confirm that all requested changes have been addressed.
Code Review
Functional Tests
Does everything look good? Mark the review as Approve. A member of
@mantidproject/gatekeepers
will take care of it.Gatekeeper
If you need to request changes to a PR then please add a comment and set the review status to "Request changes". This will stop the PR from showing up in the list for other gatekeepers.