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

7.185.0 Deletes __tests__ Folder although listed in .foreceignore #1904

Closed
GH4Arlene opened this issue Jan 30, 2023 · 10 comments
Closed

7.185.0 Deletes __tests__ Folder although listed in .foreceignore #1904

GH4Arlene opened this issue Jan 30, 2023 · 10 comments
Labels
bug Issue or pull request that identifies or fixes a bug

Comments

@GH4Arlene
Copy link

Summary

Using the same .forceignore file as was used with version 7.184 and previous, sfdx source:force:retrieve with either the -m or -p flag deletes local tests folders and the tests within them.

Steps To Reproduce:

  1. Make sure .foreignore is ignoring tests:
    image

  2. Create a LWC with a test and save it to the org:
    image

  3. Retrieve the source:
    image

Expected result

The Retrieved Source list should not have listed the tests folder and the tests folder and contents should have been left in place. This was the behavior prior to 7.185.

Actual result

The tests folder and its test have been deleted:
image

System Information

shell: cmd

C:\Users\stebbinsa\vsworkspace\myhhtqa>sfdx version --verbose --json
{
"cliVersion": "sfdx-cli/7.185.0",
"architecture": "win32-x64",
"nodeVersion": "node-v18.12.1",
"pluginVersions": [
"@oclif/plugin-autocomplete 1.3.10 (core)",
"@oclif/plugin-commands 2.2.2 (core)",
"@oclif/plugin-help 5.1.22 (core)",
"@oclif/plugin-not-found 2.3.13 (core)",
"@oclif/plugin-plugins 2.1.8 (core)",
"@oclif/plugin-search 0.0.8 (core)",
"@oclif/plugin-update 3.0.9 (core)",
"@oclif/plugin-version 1.1.4 (core)",
"@oclif/plugin-warn-if-update-available 2.0.19 (core)",
"@oclif/plugin-which 2.2.6 (core)",
"alias 2.1.18 (core)",
"apex 1.4.2 (core)",
"auth 2.3.12 (core)",
"community 2.1.3 (core)",
"config 1.4.23 (core)",
"custom-metadata 2.0.11 (core)",
"data 2.1.22 (core)",
"generator 2.0.16 (core)",
"info 2.3.3 (core)",
"limits 2.2.3 (core)",
"org 2.2.22 (core)",
"packaging 1.12.3 (core)",
"schema 2.2.2 (core)",
"signups 1.2.12 (core)",
"source 2.3.15 (core)",
"telemetry 2.0.5 (core)",
"templates 55.2.1 (core)",
"trust 2.2.8 (core)",
"user 2.1.25 (core)",
"@salesforce/sfdx-plugin-lwc-test 1.0.1 (core)",
"salesforce-alm 54.8.5 (core)"
],
"osVersion": "Windows_NT 10.0.19045",
"shell": "cmd.exe",
"rootPath": "C:\Users\stebbinsa\AppData\Roaming\npm\node_modules\sfdx-cli"

@GH4Arlene GH4Arlene added the investigating We're actively investigating this issue label Jan 30, 2023
@github-actions
Copy link

Thank you for filing this issue. We appreciate your feedback and will review the issue as soon as possible. Remember, however, that GitHub isn't a mechanism for receiving support under any agreement or SLA. If you require immediate assistance, contact Salesforce Customer Support.

@shetzel shetzel added bug Issue or pull request that identifies or fixes a bug and removed investigating We're actively investigating this issue labels Jan 30, 2023
@git2gus
Copy link

git2gus bot commented Jan 30, 2023

This issue has been linked to a new work item: W-12460543

@shetzel
Copy link
Contributor

shetzel commented Jan 30, 2023

This is a regression from when we added support for retrieving partial bundle deletes. We'll get this fixed and added to the upcoming release candidate. Apologies for the regression and thanks for reporting!

@GH4Arlene
Copy link
Author

Thank you much!

@shetzel
Copy link
Contributor

shetzel commented Feb 3, 2023

This should be fixed in the release candidate version of the CLI, 7.187.1. Feel free to try it out. There is still a bug that will be fixed asap with these conditions:

  1. an LWC with a non-forceignored file removed in the org
  2. that removed file exists locally for a developer retrieving the changes
  3. there is a forceignored directory (e.g., tests) locally

If you see this, know that the fix is coming in the next RC.

@ethan-sargent
Copy link

Maybe I'm missing what you mean by the remaining bug listed above, but I've installed the release candidate and I'm still losing files with 7.187.1.

Steps to reproduce:

  1. Add **/package.json to forceignore
  2. Create package.json file in LWC folder (touch path/to/lwc/package.json)
  3. Retrieve LWC from org using force:source:retrieve. package.json disappears and is displayed in list of "retrieved files".

I also continue to lose tests directories despite not seeing tests reported as retrieved after force:source:retrieve.

7.184 continues to work as expected.

@shetzel
Copy link
Contributor

shetzel commented Feb 7, 2023

@ethan-sargent - it's another bug. The implementation will have to be changed. Thanks for posting.

@shetzel
Copy link
Contributor

shetzel commented Feb 9, 2023

@ethan-sargent @GH4Arlene - the latest-rc release of the CLI (7.188.0) should have everything fixed. Please let me know (and open this up again) if not.

@ethan-sargent
Copy link

hey @shetzel sorry for the delay on this, had just been running on 7.184 for the past couple of weeks, and have had time to retest this on latest-rc (7.189.1) which does resolve this as per the description in your linked PR forcedotcom/source-deploy-retrieve#847

TLDR below: Should retrieving be a "safe" operation for files not present in the org?

Not sure if this is worth reopening, however in 7.184 it didn't require explicitly forceignoring files in an lwc folder in order to avoid overwriting them on retrieve, only to ignore them on deploy.

Very minor impact, but I'm not sure whether retrieving should ever destroy local files that aren't present in the org - other examples of this would be retrieving force-app/main/default/profiles, where a profile in source isn't present in the org - the CLI outputs an error as it can't find the source, but it doesn't delete the profile file if not found.

eg If I retrieve force-app/main/default/profiles where a local profile Test_Missing_Profile exists but isn't present in the org, this displays after the successfully retrieved profiles:

=== Retrieved Source Warnings

FILE NAME PROBLEM
────────────────────── ──────────────────────────────────────────────────────────────────
unpackaged/package.xml Entity of type 'Profile' named 'Test_Missing_Profile' cannot be found

I believe ExperienceBundles also preserve old views/routes on retrieve, rather than erase files that aren't present in the org any longer. I haven't tested this recently though so this may be incorrect/out of date.

@shetzel
Copy link
Contributor

shetzel commented Feb 23, 2023

@ethan-sargent - re: Should retrieving be a "safe" operation for files not present in the org?

Great question. Arguments could be made either way, but in the end we felt that the most correct action was to have the local component match the retrieved component while still respecting force ignored files. Note that this behavior only applies to components that are "bundle types" as defined in the client side library's metadata registry here. Look for entries with supportsPartialDelete. Profiles would not be affected. Without these changes, developer 1 could remove a css file from an LWC; developer 2 would retrieve that LWC, the css file would not be removed, developer 2 could make some changes to the LWC and deploy, and the css file would be back. Other examples are translations of DigitalExperienceBundle pages; if a translation language is removed, all the translation files would remain after a retrieve. If a source control system is being used locally it could tell you either way, but with these changes it makes it more obvious from the CLI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue or pull request that identifies or fixes a bug
Projects
None yet
Development

No branches or pull requests

4 participants