Skip to content

Integration tests fixing alt home#26920

Merged
PVince81 merged 3 commits into
masterfrom
integration-tests-fixing-alt-home
Jan 11, 2017
Merged

Integration tests fixing alt home#26920
PVince81 merged 3 commits into
masterfrom
integration-tests-fixing-alt-home

Conversation

@SergioBertolinSG

Copy link
Copy Markdown
Contributor

Fixing the problems found in #26918

Running tests locally, will add output after they finish.

@mention-bot

Copy link
Copy Markdown

@SergioBertolinSG, thanks for your PR! By analyzing the history of the files in this pull request, we identified @PVince81, @DeepDiver1975 and @davitol to be potential reviewers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

with "home" do you mean local folder on the local machine, not ownCloud home ?
This wording is confusing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, I mean owncloud home. User is also an owncloud user.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This scenario as worded is not possible in a regular ownCloud. You can't upload a file from your ownCloud home. You can upload to your ownCloud home.

I think the reason for this is that you're reading the file directly from inside the data folder to test upload ? You should rather read the file from an external folder containing sample files.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also one problem with reading a file directly from the data folder is that in some cases that file could be encrypted, that could break tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK, then I'll have to change those scenarios.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Moving the file seems more appropiate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have changed the federation tests, now that part is not needed.

@PVince81 PVince81 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good except for the wording issue

@SergioBertolinSG

Copy link
Copy Markdown
Contributor Author

Tests using alt home variable have passed.

@SergioBertolinSG SergioBertolinSG force-pushed the integration-tests-fixing-alt-home branch from a9197f2 to 6c2f308 Compare January 11, 2017 16:27
@SergioBertolinSG

Copy link
Copy Markdown
Contributor Author

Tests have passed locally after last changes.

@PVince81

Copy link
Copy Markdown
Contributor

Thanks, merging then 👍

I'll then rebase the other PR after that.

@lock

lock Bot commented Aug 4, 2019

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock Bot locked as resolved and limited conversation to collaborators Aug 4, 2019
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.

3 participants