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

Expand pkStdio, pkExec and pkSpawn utils to target executables #410

Closed
6 tasks done
tegefaulkes opened this issue Jul 12, 2022 · 4 comments · Fixed by #391
Closed
6 tasks done

Expand pkStdio, pkExec and pkSpawn utils to target executables #410

tegefaulkes opened this issue Jul 12, 2022 · 4 comments · Fixed by #391
Assignees
Labels
development Standard development r&d:polykey:supporting activity Supporting core activity

Comments

@tegefaulkes
Copy link
Contributor

tegefaulkes commented Jul 12, 2022

Specification

We need to expand the pkStdio, pkExec, and pkSpawn utility commands to support testing executable files such as the win, mac builds and docker container. Counterparts to these commands need to be created. These new commands need to mimic their counterparts but execute the target executable.

Additional context

Tasks

  1. Create counterpart commands that execute the targeted executable
    • pkExecTarget
    • pkStdioTarget - this can't work with mocking so keep that in mind.
    • pkSpawnTarget
  2. add switching utility functions that will use a pk command or it's counterpart to execute the command.
  3. add code docs to each command clearly specifying usage.
@tegefaulkes tegefaulkes added the development Standard development label Jul 12, 2022
@tegefaulkes tegefaulkes self-assigned this Jul 12, 2022
tegefaulkes added a commit that referenced this issue Jul 12, 2022
This is to be used with the pkg integration tests to test the docker, windows and mac executables.

Related #410
@tegefaulkes
Copy link
Contributor Author

tegefaulkes commented Jul 12, 2022

pkXTarget commands explicitly take the command now. global.testCmd needs to be passed into it. I've also added pkXSwitch commands that will switch between the normal and target version of the pk commands. Used like so.

pkSpawn(args, env, cwd, logger);
// becomes
pkSpawnSwitch(global.testCmd)(args, env, cwd, logger);

@CMCDragonkai
Copy link
Member

And ultimately the global.testCmd is specified through an environment variable?

Would it be possible that the test code automatically specifies the testCmd depending on the "testing platform", but that it can be overwritten by the env variable if it is set.

@tegefaulkes
Copy link
Contributor Author

I think @emmacasolin had a way to check what platform was when tests were running. I'll have to dig through the NAT tests for that. Either way we can't do this for the docker tests.

tegefaulkes added a commit that referenced this issue Jul 12, 2022
These return either the pkX or the pkXTarget command if the `cmd` is set.

Related #410
@CMCDragonkai
Copy link
Member

I think you should keep the entrypoint functions the same name as before. It should be just pkExec and pkSpawn as before.

But internally switch to a different implementation.

You can do this like:

function pkExec() {
  if (...) { return pkExecTarget(); }
  // ... original functionality ...
}

function pkExecTarget() {
  // new functionality
}

tegefaulkes added a commit that referenced this issue Jul 26, 2022
This is to be used with the pkg integration tests to test the docker, windows and mac executables.

Related #410
tegefaulkes added a commit that referenced this issue Jul 26, 2022
These return either the pkX or the pkXTarget command if the `cmd` is set.

Related #410
tegefaulkes added a commit that referenced this issue Jul 27, 2022
This is to be used with the pkg integration tests to test the docker, windows and mac executables.

Related #410
tegefaulkes added a commit that referenced this issue Jul 27, 2022
These return either the pkX or the pkXTarget command if the `cmd` is set.

Related #410
tegefaulkes added a commit that referenced this issue Jul 27, 2022
`DOCKER_OPTIONS` env variable containing all the docker run parameters to make the test work is provided to the command.

#410
tegefaulkes added a commit that referenced this issue Jul 27, 2022
…time

The `runTestIfPlatform` commands will default to running if `PK_TEST_PLATFORM` is not set.

#410
tegefaulkes added a commit that referenced this issue Jul 27, 2022
`DOCKER_OPTIONS` env variable containing all the docker run parameters to make the test work is provided to the command.

#410
tegefaulkes added a commit that referenced this issue Jul 27, 2022
…time

The `runTestIfPlatform` commands will default to running if `PK_TEST_PLATFORM` is not set.

#410
@CMCDragonkai CMCDragonkai added the r&d:polykey:supporting activity Supporting core activity label Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development r&d:polykey:supporting activity Supporting core activity
Development

Successfully merging a pull request may close this issue.

2 participants