osl-incubator / makim

Make Improved
https://osl-incubator.github.io/makim/
BSD 3-Clause "New" or "Revised" License
8 stars 10 forks source link

feat: Add working-directory to the target, group and global scope #65

Closed abhijeetSaroha closed 10 months ago

abhijeetSaroha commented 11 months ago

Changes Made:

Details:

Solves #48

abhijeetSaroha commented 11 months ago

Heyy @xmnlab , can you please review this?

abhijeetSaroha commented 11 months ago
xmnlab commented 11 months ago

@abhijeetSaroha could you please create some tests? you can create a new makim file for test based on this other one: https://github.com/osl-incubator/makim/blob/main/tests/.makim-env.yaml

it could be tests/.makim-working-directory.yaml

some ideas for the tests (pseudo makim file, not the real one):

global:
- working directory: /tmp
- groups:
  group-setup:
    working directory: group1
    targets:
      target-setup:
        run: |
          # create here all the paths necessary for the test ex:
          mkdir -p /tmp/group1
  group1:
   targets:
    target-1:  # NO  working directory
       run: |
          import os
          assert os.getcwd() == "/tmp/group1"

    target-2 working directory: target2
      run: |
        import os
        assert os.getcwd() == "/tmp/group1/target2"

this is just an idea of example, try to create different kind of tests for the following states:

and you should combine this for all the 3 scopes example

maybe you will need to have 3 makim files for that because the 3 different states for the global path

Create a new target here https://github.com/osl-incubator/makim/blob/main/.makim.yaml#L87 for the working-directory test, in the beginning of your test call the target for the setup, so it will create all the directory first

additionally, update this target in order to add your new targets for working directory smoke tests: https://github.com/osl-incubator/makim/blob/main/.makim.yaml#L69C7-L69C12

xmnlab commented 11 months ago

additionally, please create a new step in the CI workflow for testing the working directory feature. example: https://github.com/osl-incubator/makim/blob/main/.github/workflows/main.yaml#L86-L87

abhijeetSaroha commented 11 months ago

Update the code as we have discussed in that last coding session.

abhijeetSaroha commented 11 months ago

/tests/,makim-working-directory-absolute.yaml

version: 1.0
working-directory: "/tmp/tests"

groups:
  group-relative:
    working-directory: "group1"
    targets:
      target-no-path:
        help: Test global absolute path, group relative path, target no path
        run: |
          import os
          assert os.getcwd() == "/tmp/tests/group1"

      target-absoulte:
        working-directory: "/tmp"
        help: Test global absolute path, group relative path, target absolute path
        run: |
          import os
          assert os.getcwd() == "/tmp"

      target-realtive:
        working-directory: "target3"
        help: Test global absolute path, group relative path, target relative path
        run: |
          import os
          assert os.getcwd() == "/tmp/tests/group1/target3"

      target-no-path:
        help: Test global absolute path, group relative path, target no path
        run: |
          import os
          assert os.getcwd() == "/tmp/tests/group1"

  group-absolute:
    working-directory: "/tmp/tests/group2"
    targets:
      target-no-path:
        help: Test global absolute path, group absolute path, target no path
        run: |
          import os 
          assert os.getcwd() == "/tmp/tests/group2"

  group-no-path:
    targets:
      target-no-path:
        help: Test global absolute path, group no path, target no path
        run: |
          import os
          assert os.getcwd() == "/tmp/tests"

      target-absolute:
        working-directory: "/tmp"
        help: Test global absolute path, group no path, target absolute path
        run: |
          import os
          assert os.getcwd() == "/tmp"

      target-relative:
        working-directory: "group1/target3"
        help: est global absolute path, group no path, target relative path
        run: |
          import os
          assert os.getcwd() == "/tmp/tests/group1/target3"

Can you please review this, what are your thoughts about this?

I know run command is almost same for everyone, but I think for testing, it will be good.

Any other suggestion!

abhijeetSaroha commented 11 months ago

Mistakenly press Close with comment instead of Comment.

abhijeetSaroha commented 11 months ago

I decided to create three files based on these combinations.

  1. tests/.makim-working-directory-absolute.yaml:

    * Global Absolute Path, Group Relative Path, Target No Path
    * Global Absolute Path, Group Relative Path, Target Absolute Path
    * Global Absolute Path, Group Relative Path, Target Relative Path
    * Global Absolute Path, Group Absolute Path, Target No Path
    * Global Absolute Path, Group No Path, Target No Path
    * Global Absolute Path, Group No Path, Target Absolute Path
    * Global Absolute Path, Group No Path, Target Relative Path
  2. tests/.makim-working-directory-relative.yaml:

    * Global Relative Path, Group Relative Path, Target No Path
    * Global Relative Path, Group Relative Path, Target Absolute Path
    * Global Relative Path, Group Relative Path, Target Relative Path
    * Global Relative Path, Group Absolute Path, Target No Path
    * Global Relative Path, Group No Path, Target No Path
    * Global Relative Path, Group No Path, Target Absolute Path
    * Global Relative Path, Group No Path, Target Relative Path
  3. tests/.makim-working-directory-no-path.yaml:

    * Global No Path, Group Relative Path, Target No Path
    * Global No Path, Group Relative Path, Target Absolute Path
    * Global No Path, Group Relative Path, Target Relative Path
    * Global No Path, Group Absolute Path, Target No Path
    * Global No Path, Group No Path, Target No Path
    * Global No Path, Group No Path, Target Absolute Path
    * Global No Path, Group No Path, Target Relative Path
xmnlab commented 11 months ago

@abhijeetSaroha you can have 3 tests for absolute path for group as well (instead of just one).

for example:

the same thing for the other configuration for global as well

xmnlab commented 11 months ago

hey @abhijeetSaroha , it looks very nice, you are in right direction! please add a step in the CI workflow for your new test, similar to this one: https://github.com/osl-incubator/makim/blob/main/.github/workflows/main.yaml#L86-L87

abhijeetSaroha commented 11 months ago

@xmnlab, It shows error for not finding the file / directory, which I have used and created for testing the working-directory-absolute-path. So, now I have to upload those directory also? or Is there any solution which should I approach or may I change the directory names in the testing file ?

xmnlab commented 10 months ago

@abhijeetSaroha as we discused before, we need a setup step before the tests. So maybe the easiest way for now, wpuld be create all the folders used by your tests in your in the .makim.yaml file inside the target you created at the beginning of the "run".

Use this example as reference: "mkdir -p /path/to/folder/"

Let me know if you need more explanation.

Thanks!!

abhijeetSaroha commented 10 months ago

@xmnlab , Now, It's creating problem different for macos and ubuntu. In macos testing, it shows that parent directory is /private but in linux, that's not the case.

xmnlab commented 10 months ago

@abhijeetSaroha , instead of using /private, use /tmp

the code for setup should go to a specific target, you can call that setup and call that as a dependency for each other target.

let me know if you need more details

xmnlab commented 10 months ago

you can have the setup in a group called setup and the target could be called folders

xmnlab commented 10 months ago

@abhijeetSaroha now it is just missing some documentation please create a new file at https://github.com/osl-incubator/makim/tree/main/docs it could be called features.md let use that to describe all the new features

You can add the following information (just an idea of structure, feel free to improve that):


# Working Directory

attribute: working-directory
scopes: global, group, target

## Example

```yaml

version: 1.0
working-directory: /tmp/tests

groups:
  group1:
    working-directory: group1
    targets:
      target-no-path:
        help: example of the usage of working-directory attribute
        working-directory: target1
        run: |
          import os
          print(os.getcwd())
abhijeetSaroha commented 10 months ago

@xmnlab If you have time, please review the features.md file.

xmnlab commented 10 months ago

Looks really nice. We don't need the section "Benefits". Could you remove that please? Other than that, It looks great.

abhijeetSaroha commented 10 months ago

Removed the section of Benefit

github-actions[bot] commented 10 months ago

:tada: This PR is included in version 1.9.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: