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

New test for File system saveas #17781

Conversation

Enzo-Demeulenaere
Copy link
Contributor

@Enzo-Demeulenaere Enzo-Demeulenaere commented Feb 6, 2025

Hello,

This Is referencing issue #17549,

We tried to fix this issue with @demarey during sprint and here is what we understood and tried to implement :

The first error says a directory does not exist so we tried to ensure that if the fileReference passed through the code corresponds to a file, its parent directory will be returned. This is done in UITheme>>chooseForSaveFileReferenceIn:title:extensions:path:preview:
This was recently fixed in merge #17767

We then wrote a test (#testCanUseFileInChooseForSaveFileReference) to save a file as a a name already used, and check if a new file with a 'duplicated' name existed.

We figured that the widget opening when doing the 'save as' operation is linked to the machine file system although we used an artificial file system for our test, that's why we added a little snippet to ensure the widget is correctly linked to the artificial file system if we use one. This is added as a suggestion on what could be done although this might break some thing if implemented as such, comment can be seen in StDirectoryTreePresenter>>expandPath: of this PR on NewTools, also this fix is needed for the test to pass so this test might be modified later when a correct implementation is found.

After those changes, we can see the widget works fine for the test although the name suggested for the new file saved is the same as the current image so this might need to be changed by default to have something like 'a.1.image' for example.

Attention, the code suggested in this PR is required for the 'save as' widget when launching the test

Also, for now the test doesn't pass automatically as it waits for an answer from the widget, take this PR as a suggestion we can discuss

@guillep
Copy link
Member

guillep commented Feb 7, 2025

Hi @Enzo-Demeulenaere , thanks for the idea.

Indeed it would be nice to have these kind of integration tests.
Notice however that a working test would be better :)

For that, it would be nice to have somehow support to test modal windows, which here is this case.
The thing with modals is that if they open from a ui thread (here is the case when you run the test from calypso or the test runner, but not from the command line), they should suspend the execution and only return after the modal is closed.
In a test, however, you want to still have control after the modal window is opened.

Now, this PR is not integratable as it is... I propose to close it, and if you have a follow up we can reopen it.

@guillep guillep closed this Feb 7, 2025
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