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

RCORE-2222 Remove 308 redirect tests #7994

Merged
merged 3 commits into from
Aug 30, 2024
Merged

Conversation

michael-wb
Copy link
Contributor

What, How & Why?

Since the redirection 301/308 support is being removed, the tests are no longer needed. The first part of these changes is to remove the redirect tests.

Fixes #7942

☑️ ToDos

  • [ ] 📝 Changelog update
  • 🚦 Tests (or not relevant)
  • [ ] C-API, if public C++ API changed
  • [ ] bindgen/spec.yml, if public C++ API changed

@michael-wb michael-wb self-assigned this Aug 23, 2024
@cla-bot cla-bot bot added the cla: yes label Aug 23, 2024
Copy link

coveralls-official bot commented Aug 23, 2024

Pull Request Test Coverage Report for Build michael.wilkersonbarker_1363

Details

  • 157 of 161 (97.52%) changed or added relevant lines in 1 file are covered.
  • 134 unchanged lines in 21 files lost coverage.
  • Overall coverage decreased (-0.05%) to 91.095%

Changes Missing Coverage Covered Lines Changed/Added Lines %
test/object-store/sync/app.cpp 157 161 97.52%
Files with Coverage Reduction New Missed Lines %
src/realm/array_backlink.cpp 1 91.38%
src/realm/object-store/sync/app_utils.cpp 1 96.15%
src/realm/sort_descriptor.cpp 1 94.06%
src/realm/sync/noinst/server/server_history.cpp 1 62.92%
test/object-store/sync/app.cpp 1 98.87%
test/test_dictionary.cpp 1 99.83%
src/realm/list.cpp 2 87.37%
src/realm/object-store/sync/sync_session.cpp 2 91.84%
src/realm/util/uri.cpp 2 96.32%
test/object-store/util/test_file.cpp 2 87.06%
Totals Coverage Status
Change from base Build 2586: -0.05%
Covered Lines: 217172
Relevant Lines: 238402

💛 - Coveralls

@jbreams
Copy link
Contributor

jbreams commented Aug 27, 2024

Should we add a test that makes the synchronous CURL http lib follow redirects, and setup a redirecting server the issues a 308 and make sure we can sync with that?

@michael-wb
Copy link
Contributor Author

Should we add a test that makes the synchronous CURL http lib follow redirects, and setup a redirecting server the issues a 308 and make sure we can sync with that?

I would be happy to do this in a separate PR - the test that I am adding for the CPP SDK will also be doing this.

@michael-wb michael-wb merged commit 07d57de into master Aug 30, 2024
45 checks passed
@michael-wb michael-wb deleted the mwb/remove-308-tests branch August 30, 2024 14:48
@github-actions github-actions bot mentioned this pull request Sep 20, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove 308 redirect support from App/AppUser
3 participants