Make the command testing script suitable for local dev use

[shalithasuranga]

Currently, it's implemented by focusing on the CI workflow, But, developers can use this script to verify breaking changes locally as well. Therefore, we need to:

• Use .tmp/ to create all test files
• Delete .tmp/ after testing all commands
• Try to remove the sudo command

[NethmiRodrigo]

Hey @shalithasuranga can I have a try at this?

[pathange-s]

@shalithasuranga May I know if this is related to test_commands.sh ?

[shalithasuranga]

Hey @pathange-s yeah, I think devs can use it locally during development to identify command breakdowns. We can assign @NethmiRodrigo to this one. Hope @pathange-s will review her pull request since you've initially implemented the script. Thanks

[pathange-s]

Cool, Will review. So currently testNeuCLI directory is made and all test related files are generated here. So, this issue means to update the script to delete this directory ?

[NethmiRodrigo]

Hey @shalithasuranga @pathange-s correct me if I'm wrong but because we have commands like xvfb-run and we install and build into /dev/null (pls correct me if I've misunderstood this), can we use this same bash script locally?

My two cents on this (I might be very wrong here or maybe this is what @shalithasuranga meant so please excuse me): Could we take in an optional argument when running the script (or some other way to check if the current environment is development or production) and then have a separate portion or maybe a completely different script for local development?

[NethmiRodrigo]

Or is this a windows specific error?

[NethmiRodrigo]

I also found a small issue with the npm run test command. Shouldn't the command be "bash scripts/test_commands.sh"? I tested it out, and this is the command that worked for me.

[pathange-s]

Or is this a windows specific error?

These commands are linux specific, and since the workflow was running on ubuntu I wrote scripts for the same. I did mail GitHub actions regarding options available to test GUI app on windows instance, waiting for a reply from the other end. To test (in case you have a windows os) what you can do is, create a test-pr pull request on the forked repository and test it there. GitHub actions are free of cost and you could do this before making an actual PR, atleast for now.

[pathange-s]

I also found a small issue with the npm run test command. Shouldn't the command be "bash scripts/test_commands.sh"? I tested it out, and this is the command that worked for me.

Same, We can use relative path reference on linux using ./ path. So, this worked fine for me, and on GitHub actions workflow too.

[pathange-s]

I also found a small issue with the npm run test command. Shouldn't the command be "bash scripts/test_commands.sh"? I tested it out, and this is the command that worked for me.

I guess this would throw error when running tests in the workflow, anyways you can try it out once.

[NethmiRodrigo]

Or is this a windows specific error?

These commands are linux specific, and since the workflow was running on ubuntu I wrote scripts for the same. I did mail GitHub actions regarding options available to test GUI app on windows instance, waiting for a reply from the other end. To test (in case you have a windows os) what you can do is, create a test-pr pull request on the forked repository and test it there. GitHub actions are free of cost and you could do this before making an actual PR, atleast for now.

I get what you mean, but what I understood from what @shalithasuranga said is, to enable using the test commands so devs can test locally, which in that case this bash script cannot be used right? What I was suggesting was changes to allow devs to test locally on any OS.

[NethmiRodrigo]

Let me clarify, what I'm suggesting is keeping the current test script for the workflow separate and having a separate script for devs to test locally.

[pathange-s]

Cool, Gotcha ! You can start working on it ! Also, let me know how will you be running the shell scripts (on windows and macOS)

[NethmiRodrigo]

Cool, Gotcha ! You can start working on it ! Also, let me know how will you be running the shell scripts (on windows and macOS)

Will do. macOS i'll probably have to ask someone on discord to test. Btw do you mean the current bash script issue I mentioned or whatever I introduce to test locally?

[pathange-s]

As you can see here the current bash script runs only on ubuntu First, let me guide you to update the workflow for windows. Then, later we can try implementing to run on macOS also. The same goes for local test dev too.

[NethmiRodrigo]

As you can see here the current bash script runs only on ubuntu First, let me guide you to update the workflow for windows. Then, later we can try implementing to run on macOS also. The same goes for local test dev too.

Hold on, do we need to update the workflows for separate OS's? For local test dev, is a workflow really necessary? Isn't it kind of troublesome to have to run an entire workflow just to test locally? I was thinking along the lines of making a script that doesn't require running any workflows, just a simple command that runs the test locally.

[pathange-s]

If you check the .yml file of the main repo, you would find that the workflow runs on Linux and Darwin and for Windows, it is yet to be updated to run the tests.

So, for different OS, there will be different set of commands defined in the .yml file. For local dev, developers usually test the changes by running main command of the workflow, for example, npm test This would test the changes made locally.

[NethmiRodrigo]

So, for different OS, there will be different set of commands defined in the .yml file. For local dev, developers usually test the changes by running main command of the workflow, for example, npm test This would test the changes made locally.

What I am suggesting is allowing devs to test locally without needing a workflow at all (like how we run unit tests). For example, running npm test would run a simple script in the scripts folder that does the same job as test_commands.sh, except locally and isn't dependent on running a workflow. This would let the devs test their code before making commits or PR's

[pathange-s]

Yes, thats what I mentioned above.

For local dev, developers usually test the changes by running main command of the workflow, for example, npm test This would test the changes made locally.

[NethmiRodrigo]

Okay wait I think we've misunderstood each other. What I'm talking about has no relation nor would affect the workflow.

[pathange-s]

We can get on a meet to discuss if texts are creating frequency mismatch

[NethmiRodrigo]

We can get on a meet to discuss if texts are creating frequency mismatch

Yeah sure, that would be better I guess. I think its better if @shalithasuranga could join in too, if free.

[Ayman161803]

Hey @NethmiRodrigo @pathange-s! I can be wrong but I think what @shalithasuranga expected was a simple rename of testNeuCLI to .tmp along with removal of sudo privileges wherever necessary.

Here is probably what is expected in a PR which might solve this issue.

[NethmiRodrigo]

Hey @NethmiRodrigo @pathange-s! I can be wrong but I think what @shalithasuranga expected was a simple rename of testNeuCLI to .tmp along with removal of sudo privileges wherever necessary.

Here is probably what is expected in a PR which might solve this issue.

Hi. Thanks for your input! Yes, what you mentioned was partly correct. This script was written for the GitHub workflow, so in addition to the folder naming change, some command changes needs to be done as well to make it compatible for local dev testing on any operating system.

[HasinduLanka]

@pathange-s @NethmiRodrigo I've made pull request #148 to make some progress for this issue. Can you look into it?

[NethmiRodrigo]

Unassigning myself from this issue since @HasinduLanka is working on it. @shalithasuranga hope you can update :)

[shalithasuranga]

Closing this since now we use a new test suite. Thanks all for working on this :tada: