Skip to content

Conversation

@bbockelm
Copy link
Collaborator

Done for @osschar and @alja to show crash reported in #2450.

This segfaults when run with address sanitizer. I assume Matevz can push fixes to actually make check for correct values.


TEST(XrdPfcTests, PathTokenizer)
{
PathTokenizer("/pelican/hello_world.txt", 0, true);
Copy link
Member

Choose a reason for hiding this comment

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

I think this would be a lot simpler if you used loops (see XrdClURL test).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes - I assume that Alja and Matevz will expand it into a larger test by pushing to the branch and not using as-is. Just wanted to help show the crash.

@osschar
Copy link
Contributor

osschar commented Mar 15, 2025

Sorry for being stoopid, but how is one supposed to run the tests? :)

I tried, in my build:

matevz@black build> make test 
Running tests...
Test project /home/matevz/xrd-blk/build
No tests were found!!!

and then, from the source directory

matevz@black xrootd> ctest -VV -S test.cmake
.... builds the full thing ...
[100%] Built target xrdreplay
Command exited with the value: 0
MakeCommand:/usr/bin/cmake --build . --config "RelWithDebInfo"
   0 Compiler errors
   0 Compiler warnings
SetCTestConfiguration:BuildDirectory:/home/matevz/xrd-blk/xrootd/build
SetCTestConfiguration:SourceDirectory:/home/matevz/xrd-blk/xrootd
Test project /home/matevz/xrd-blk/xrootd/build
Constructing a list of tests
Done constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
No tests were found!!!

@amadio
Copy link
Member

amadio commented Mar 15, 2025

@osschar You are likely missing dependencies to run the tests (see summary at configuration step). If you are on an RPM-based distro, try dnf builddep -y xrootd.spec at the top directory of the repository to install the missing deps. For server tests, you should not run as the root user. If you want to run in docker, you can cd docker/ && xrd-docker build alpine, for example.

@osschar
Copy link
Contributor

osschar commented Mar 15, 2025

Thank you @amadio, this helped! :)

@osschar
Copy link
Contributor

osschar commented Mar 16, 2025

Thank you @bbockelm! If you both agree, I'll cherry-pick this and put it on my "fix-2450" branch along with improvements to to the unit test, as requested by @amadio.

@bbockelm
Copy link
Collaborator Author

A cherry-pick sounds good! As @amadio mentions, you'll probably want to flesh out the test to check the actual functionality of the class beyond "it doesn't crash".

(Closing for now since it'll be in the other branch)

@osschar
Copy link
Contributor

osschar commented Mar 17, 2025

Included in PR #2455.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants