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.

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

Add unit tests for subdirectory lookup #3

Open
wants to merge 1 commit into
base: eff_subdirs
Choose a base branch
from

Conversation

asarium
Copy link

@asarium asarium commented Nov 20, 2016

No description provided.

@asarium asarium force-pushed the eff_subdirs_unit_tests branch from c7c93cd to 45ee0b2 Compare December 22, 2016 19:04
class CFileTest : public test::FSTestFixture {
public:
CFileTest() : test::FSTestFixture(0) {
pushModDir("cfile");
Copy link
Owner

Choose a reason for hiding this comment

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

I don't understand the hows and whys, but there seems to be something wrong with the ways directories are handled in these tests. Changing this line to pushModDir("cfile/temp_subdir") makes the temp_subdir tests work, and changing it to pushModDir("cfile/temp_subdir_vps") makes the temp_subdir_vps tests work. Added debugging statements say that all file lookup happens relative to test_data/cfile/, not relative to test_data/cfile/temp_subdir/ and test_data/cfile/temp_subdir_vps/ as it should.

However, I don't know why for example adding a test like ASSERT_TRUE(cf_exists_full("test.eff", CF_TYPE_ANY)) doesn't cause a(nother) failure. Debug output tells me that it looks in the wrong path (as described above), yet that doesn't cause the test to fail.

Copy link
Author

Choose a reason for hiding this comment

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

pushModDir simply adds another -mod parameter so it seems like the subdirectory implementation doesn't handle mods correctly.

ln-zookeeper pushed a commit that referenced this pull request Mar 2, 2017
Merge materials branch with master
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.

2 participants