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

feat: follow symbolic link to search test case files #35

Merged
merged 1 commit into from
Feb 13, 2023

Conversation

MichaelScofield
Copy link
Contributor

Which issue does this PR close?

Closes #

Rationale for this change

Some test case files can be shared between env. Use symbolic links could avoid copying these files. walkdir crate already provides this flag when traverse the directory, we can just simply make it a config option to let users enable it. The default behavior is not follow symbolic links.

What changes are included in this PR?

Expose a "follow links" config option.

Are there any user-facing changes?

no

How does this change test

@jiacai2050
Copy link
Member

jiacai2050 commented Feb 10, 2023

Hi, Have you checked #23?

I think we can support multiple case directories to solve this issue, something like this

$ tree
.
├── case1
│   ├── cluster
│   └── local
├── common1
└── common2

And we can build config with

    let config = ConfigBuilder::default()
        .case_dir("examples/basic-case".to_string())
        .no_env_case_dir(vec!["common1".to_string(), "common2".to_string()])
        .build()
        .unwrap();

There are no env directory inside common1, common2, and testcases inside them are shared by cluster and local.

How do you think of this proposal?

@waynexia
Copy link
Member

How to config when some cases are not that "common"? Like common case 1, 2 are for env A, and case 2, 3 are for B.

I would also prefer the link way, but it's also hard to manage symlink. Maybe we should give extra marker in the shell output to mark a case is symlinked?

Or another way, how about using hard link instead? As common cases are symmetric, this looks more natural to symlink.

@jiacai2050
Copy link
Member

How to config when some cases are not that "common"? Like common case 1, 2 are for env A, and case 2, 3 are for B.

If those cases are considered, I think multiple envs supported make little sense...

Does Windows support hard/symbolic links?

@MichaelScofield
Copy link
Contributor Author

It seems that hard link does not work on directories. If using hard link, we have to manually create links for all common test files.

@waynexia
Copy link
Member

Does Windows support hard/symbolic links?

Yes.

It seems that hard link does not work on directories. If using hard link, we have to manually create links for all common test files.

Still useful in some cases I think. And it's not in conflict with symlink in this PR (we're already supported hard link I guess?). To this PR, we can default the option to true. Actually I don't think someone will add links to their test directory but doesn't want to use it.

Copy link
Member

@jiacai2050 jiacai2050 left a comment

Choose a reason for hiding this comment

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

LGTM

@jiacai2050
Copy link
Member

Merging since no obvious reason block this and everyone agree.

@jiacai2050 jiacai2050 added this pull request to the merge queue Feb 13, 2023
Merged via the queue into CeresDB:main with commit e6d602e Feb 13, 2023
@MichaelScofield MichaelScofield deleted the feat/follow-soft-link branch February 13, 2023 05:59
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