Fix Imports and Add Import Tests

[phalestrivir]

Fixes a couple of different import ops, including agent, idm, and circle of trust imports. Adds a bunch of tests for most of the import ops as well, with corresponding test mocks contained in the pull request titled "Add Mocks for Test Imports" in the frodo-lib repo. Note that one of the journey import tests written did not pass and is currently being skipped for the meantime (I could not figure out why it wasn't passing).

[vscheuber]

Hi @phalestrivir :) Thanks for this one! The import tests are failing in my env due to parallel execution and the import -A/--all-separate tests are colliding as all the e2e tests execute in parallel.

I almost see not better/other way out than finally enabling all the commands to support -D and support changing the working directory at runtime so we can import from different directories at the same time vs copying to current directory which is shared by hundreds of tests running in parallel.

It might take me a while to do this unless you have cycles to take it on?

[vscheuber]

@phalestrivir I also think it's time to move the cli e2e mocks into the cli project. I'm sick of having to do a lib release just to add cli e2e tests... so expect a commit to that end, shortly.

[phalestrivir]

Hi @phalestrivir :) Thanks for this one! The import tests are failing in my env due to parallel execution and the import -A/--all-separate tests are colliding as all the e2e tests execute in parallel.

I almost see not better/other way out than finally enabling all the commands to support -D and support changing the working directory at runtime so we can import from different directories at the same time vs copying to current directory which is shared by hundreds of tests running in parallel.

It might take me a while to do this unless you have cycles to take it on?

@vscheuber Yes, sorry about that, I didn't consider the all-separate tests colliding with each other, I only considered how they might collide with the export tests (and this is something I have been unable to successfully simulate myself since the tests only run sequentially for me despite utilizing multiple CPUs), but I could definitely do some work on adding in the -D option to all the commands to help fix the problem. Another option would be to name all the files differently when they get copied between the tests so they don't collide, and I believe that would be a simpler fix since we could just change the one method that does it, but I could see the -D option being useful even outside of tests when doing exports or imports, so that may be a better option. If you haven't already started on it I could work on getting that done today and tomorrow.

[vscheuber]

Hey!

I have not started on the -D option, yet, other than what I did months ago and added a "directory" property to the State module with getter and setter functions. It appears it's only being used in two locations as of now:

% grep -R "setDirectory(" src/* src/cli/journey/journey-export.ts: if (options.directory) state.setDirectory(options.directory); src/cli/service/service-import.ts: state.setDirectory(options.directory || '.');

I'd like to come up with a consistent way for all our commands to support th -D option so we can replicate it going forward. I was hoping to add the option to all the commands that need it and then have the ops layer in the cli be aware of the state.getDirectory() value and use it or default to current directory.

I leave it up to you to determine if you want to do a quick fix for the -A tests or add value to the tool while fixing the tests. I believe the -D could be substantial effort but maybe I'm wrong and it comes easy.

Volker Scheuber 801.473.5451

On Oct 12, 2023, at 10:15 AM, phalestrivir @.***> wrote:

Hi @phalestrivir https://github.com/phalestrivir :) Thanks for this one! The import tests are failing in my env due to parallel execution and the import -A/--all-separate tests are colliding as all the e2e tests execute in parallel.

I almost see not better/other way out than finally enabling all the commands to support -D and support changing the working directory at runtime so we can import from different directories at the same time vs copying to current directory which is shared by hundreds of tests running in parallel.

It might take me a while to do this unless you have cycles to take it on?

@vscheuber https://github.com/vscheuber Yes, sorry about that, I didn't consider the all-separate tests colliding with each other, I only considered how they might collide with the export tests (and this is something I have been unable to successfully simulate myself since the tests only run sequentially for me despite utilizing multiple CPUs), but I could definitely do some work on adding in the -D option to all the commands to help fix the problem. Another option would be to name all the files differently when they get copied between the tests so they don't collide, and I believe that would be a simpler fix since we could just change the one method that does it, but I could see the -D option being useful even outside of tests when doing exports or imports, so that may be a better option. If you haven't already started on it I could work on getting that done today and tomorrow.

— Reply to this email directly, view it on GitHub https://github.com/rockcarver/frodo-cli/pull/299#issuecomment-1759809548, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG3VEUTKREF42CRMEYVDCC3X7ACRBANCNFSM6AAAAAA523NVCQ. You are receiving this because you were mentioned.

[phalestrivir]

@vscheuber Sounds good! I don't think it will be too much substantial effort to add the -D option to all commands. I have already started on it, and decided it would be best to add -D as a global option in FrodoCommand.ts, that way we can call state.setDirectory() in one place, and then anywhere that requires the directory can just call state.getDirectory(). I also created a helper method in ExportImportUtils that, given a file name, will return the file path to the file in the working directory, and will create the directories as necessary based on a boolean flag passed into the method. Most of the work now will be going through all the export/import methods in the Ops layer and calling this helper method where necessary, as well as updating the export/import tests to utilize the -D option where needed. If all goes well, I will be finished with it later today, so expect a PR for it sometime soon.