Skip to content

chore: Skip tests in server build if no changes in server files for ok-to-test command#24616

Merged
Aishwarya-U-R merged 1 commit intoreleasefrom
chore/skip-tests-in-ok-to-test-command
Jun 21, 2023
Merged

chore: Skip tests in server build if no changes in server files for ok-to-test command#24616
Aishwarya-U-R merged 1 commit intoreleasefrom
chore/skip-tests-in-ok-to-test-command

Conversation

@rajatagrawal
Copy link
Copy Markdown
Contributor

@rajatagrawal rajatagrawal commented Jun 19, 2023

Fixes #24700

These changes will allow the CI to skip tests if there are no changes in server folder when using ok-to-test command. This will save CI minutes

@github-actions github-actions bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Jun 19, 2023
@Aishwarya-U-R
Copy link
Copy Markdown
Contributor

@rajatagrawal - can we do oktotest & test this out?

@Aishwarya-U-R Aishwarya-U-R requested a review from sarojsarab June 19, 2023 10:50
@rajatagrawal
Copy link
Copy Markdown
Contributor Author

@rajatagrawal - can we do oktotest & test this out?

I am not sure how to do it, because it can only run if it is merged to release branch. I can test it if we add support for running ok-to-test command on a pull request if that is okay.

Comment thread .github/workflows/integration-tests-command.yml Outdated
@sarojsarab
Copy link
Copy Markdown
Contributor

sarojsarab commented Jun 19, 2023

@rajatagrawal - can we do oktotest & test this out?

I am not sure how to do it, because it can only run if it is merged to release branch. I can test it if we add support for running ok-to-test command on a pull request if that is okay.

@rajatagrawal I think you can update the test-build-docker-image.yml and run the workflow manually to check . Once done you can simply revert the changes before merge.

@rajatagrawal rajatagrawal force-pushed the chore/skip-tests-in-ok-to-test-command branch from 4cbdeaf to 1e615c9 Compare June 19, 2023 16:26
@rajatagrawal
Copy link
Copy Markdown
Contributor Author

I have checked out the base code now using actions/checkout@v3. I don't know if I can access the pull request number if I use test-build-docker-image.yml and dispatch the event manually ? Any other suggested workarounds @sarojsarab ?

@rajatagrawal rajatagrawal requested a review from sarojsarab June 20, 2023 11:11
@sarojsarab
Copy link
Copy Markdown
Contributor

sarojsarab commented Jun 20, 2023

I have checked out the base code now using actions/checkout@v3. I don't know if I can access the pull request number if I use test-build-docker-image.yml and dispatch the event manually ? Any other suggested workarounds @sarojsarab ?

@rajatagrawal You can hardcode the PR number.

@rajatagrawal rajatagrawal force-pushed the chore/skip-tests-in-ok-to-test-command branch from b26263b to f02943c Compare June 21, 2023 07:37
@rajatagrawal
Copy link
Copy Markdown
Contributor Author

@sarojsarab

I have tested the changes on test-build-docker-image.yml workflow.

These are the outputs of those test runs:

  1. when server changes present, skip tests false : https://github.com/appsmithorg/appsmith/actions/runs/5330910644
  2. when server changes not present, skip tests true : https://github.com/appsmithorg/appsmith/actions/runs/5330879566

@github-actions github-actions bot added the Task A simple Todo label Jun 21, 2023
@rajatagrawal rajatagrawal added the Test Plan Approved Manual/Cypress tests covers changes made on the PR. Else, add skip-testPlan label if not applicable label Jun 21, 2023
@github-actions github-actions bot added the CI label Jun 21, 2023
Copy link
Copy Markdown
Contributor

@sarojsarab sarojsarab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aishwarya-U-R
Copy link
Copy Markdown
Contributor

Aishwarya-U-R commented Jun 21, 2023

@sarojsarab

I have tested the changes on test-build-docker-image.yml workflow.

These are the outputs of those test runs:

  1. when server changes present, skip tests false : https://github.com/appsmithorg/appsmith/actions/runs/5330910644
  2. when server changes not present, skip tests true : https://github.com/appsmithorg/appsmith/actions/runs/5330879566

@rajatagrawal - Both these links are telling me Invalid workflow file?

@rajatagrawal
Copy link
Copy Markdown
Contributor Author

@Aishwarya-U-R Can you please check Attempt #1 in both those links ? I think those links have two attempts. The ones that I have tested on are Attempt #1.

@rajatagrawal
Copy link
Copy Markdown
Contributor Author

Attempt #2 is showing as missing workflow files because I moved the changes from test docker image yml file to ok-to-test yml file for the final commit.

@rajatagrawal
Copy link
Copy Markdown
Contributor Author

@Aishwarya-U-R can you please merge it also ? This PR doesn't need a CI test run.

@Aishwarya-U-R
Copy link
Copy Markdown
Contributor

Aishwarya-U-R commented Jun 21, 2023

@rajatagrawal - why is server build moved from parallel step with client build, please have this check alongside while having it at same level as client build like current:
Screenshot 2023-06-21 at 2 04 13 PM

Copy link
Copy Markdown
Contributor

@Aishwarya-U-R Aishwarya-U-R left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rajatagrawal - Have server-build parallel to client build as now.

@rajatagrawal
Copy link
Copy Markdown
Contributor Author

@Aishwarya-U-R

This diagram is from test docker build.yml which was only used for testing the workflow. The changes are actually moved
ok-to-test workflow after the changes work fine in it.

It is nonetheless parallel in test docker build yml as well, because in the workflow runs in this concerned screenshot, the server workflow ran after changeset passed. It didn't wait for client build.

I think that the diagram is showing that server has a dependency on client because all jobs i.e client/server/rds are required for the final build docker image job.

@Aishwarya-U-R
Copy link
Copy Markdown
Contributor

Ah ok, Thanks much @sarojsarab for explaining me the same.
@rajatagrawal - merging this now - please have an eye on new runs to make sure our changes are actually doing ok!

@Aishwarya-U-R Aishwarya-U-R merged commit 7ceaa9a into release Jun 21, 2023
@Aishwarya-U-R Aishwarya-U-R deleted the chore/skip-tests-in-ok-to-test-command branch June 21, 2023 09:20
rajatagrawal added a commit that referenced this pull request Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI skip-changelog Adding this label to a PR prevents it from being listed in the changelog Task A simple Todo Test Plan Approved Manual/Cypress tests covers changes made on the PR. Else, add skip-testPlan label if not applicable

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Task]: Skip tests in ok-to-test command if there are no server changes

3 participants