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

[C++][FS][Azure] Make mutations of test data explicit in azurefs_test.cc #39119

Closed
felipecrv opened this issue Dec 6, 2023 · 0 comments · Fixed by #39207
Closed

[C++][FS][Azure] Make mutations of test data explicit in azurefs_test.cc #39119

felipecrv opened this issue Dec 6, 2023 · 0 comments · Fixed by #39207

Comments

@felipecrv
Copy link
Contributor

Describe the enhancement requested

Some Filesystem tests need different initial state of the database so having PreexisingContainerName() and PreexistingObjectName() [1] as test globals prevent some test cases from having a clean slate starting point.

[1] https://github.com/apache/arrow/blob/main/cpp/src/arrow/filesystem/azurefs_test.cc#L216

Component(s)

C++

@kou kou changed the title [C++] Make mutations of test data explicit in azurefs_test.cc [C++][FS][Azure] Make mutations of test data explicit in azurefs_test.cc Dec 7, 2023
felipecrv added a commit that referenced this issue Dec 14, 2023
…antiation (#39207)

### Rationale for this change

This PR contains some unrelated improvements (like better docs) and some nitpicky fixes. The test refactoring makes it easier to see on which environments tests run and make them able to be instantiated with different options in the future once we extend the `AzureOptions`.

### What changes are included in this PR?

 - Random cleanups
 - Short namespace aliases
 - Refactoring of the tests (multiple environments, TYPED_TEST_SUITE, explicit preexisting data setup)
 - Cleanup of the `AzureOptions` class

### Are these changes tested?

Yes. I created Azure Storage accounts to test with and without Hierarchical Namespace support. I also ran the tests in a shell without my environment variables to ensure the test-skipping behavior is correct.

### Are there any user-facing changes?

Changes to `AzureOptions`, but since `AzureFileSystem` is not really used yet, these breaking changes won't be a problem.
* Closes: #39119

Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
@felipecrv felipecrv added this to the 15.0.0 milestone Dec 14, 2023
clayburn pushed a commit to clayburn/arrow that referenced this issue Jan 23, 2024
…s instantiation (apache#39207)

### Rationale for this change

This PR contains some unrelated improvements (like better docs) and some nitpicky fixes. The test refactoring makes it easier to see on which environments tests run and make them able to be instantiated with different options in the future once we extend the `AzureOptions`.

### What changes are included in this PR?

 - Random cleanups
 - Short namespace aliases
 - Refactoring of the tests (multiple environments, TYPED_TEST_SUITE, explicit preexisting data setup)
 - Cleanup of the `AzureOptions` class

### Are these changes tested?

Yes. I created Azure Storage accounts to test with and without Hierarchical Namespace support. I also ran the tests in a shell without my environment variables to ensure the test-skipping behavior is correct.

### Are there any user-facing changes?

Changes to `AzureOptions`, but since `AzureFileSystem` is not really used yet, these breaking changes won't be a problem.
* Closes: apache#39119

Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
…s instantiation (apache#39207)

### Rationale for this change

This PR contains some unrelated improvements (like better docs) and some nitpicky fixes. The test refactoring makes it easier to see on which environments tests run and make them able to be instantiated with different options in the future once we extend the `AzureOptions`.

### What changes are included in this PR?

 - Random cleanups
 - Short namespace aliases
 - Refactoring of the tests (multiple environments, TYPED_TEST_SUITE, explicit preexisting data setup)
 - Cleanup of the `AzureOptions` class

### Are these changes tested?

Yes. I created Azure Storage accounts to test with and without Hierarchical Namespace support. I also ran the tests in a shell without my environment variables to ensure the test-skipping behavior is correct.

### Are there any user-facing changes?

Changes to `AzureOptions`, but since `AzureFileSystem` is not really used yet, these breaking changes won't be a problem.
* Closes: apache#39119

Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant