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

Set gh-pages as default branch of new repo created by animint2pages #138

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

Faye-yufan
Copy link
Contributor

@Faye-yufan Faye-yufan commented Jun 28, 2024

Resolve #134
After push animint files onto Github remote repo, change the viz repo default branch to gh-pages.

@Faye-yufan Faye-yufan linked an issue Jun 28, 2024 that may be closed by this pull request
@Faye-yufan
Copy link
Contributor Author

The tests for animint2pages all passed on my local.
But in GH action, the 'PATCH' command seems to be failing due to the token in the environment.

owner = owner,
repo = repo_name
)
repo_info$default_branch == "gh-pages"
Copy link
Collaborator

Choose a reason for hiding this comment

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

this looks like a good start. Thanks.
however, I think a better test would be if you clone the repo and test if branch name of the clone is gh-pages ... That test would more directly address the usability concern (user would not have to switch branches) can you please change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea! I will do that.

@Faye-yufan Faye-yufan marked this pull request as ready for review July 2, 2024 05:51
tests/testthat/test-compiler-ghpages.R Outdated Show resolved Hide resolved
tests/testthat/test-compiler-ghpages.R Outdated Show resolved Hide resolved
@tdhock
Copy link
Collaborator

tdhock commented Jul 7, 2024

the error that we are getting on github actions is below, do you know why? how to fix?

══ Failed tests ════════════════════════════════════════════════════════════════
── Error ('test-compiler-ghpages.R:26:3'): animint2pages() returns owner/repo string ──
<github_error/http_error_403/rlang_error/error/condition>
Error in `gh::gh("PATCH /repos/{owner}/{repo}", owner = owner, repo = gh_repo_name, 
    default_branch = "gh-pages")`: GitHub API error (403): Resource not accessible by personal access token
ℹ Read more at <https://docs.github.com/rest/repos/repos#update-a-repository>
Backtrace:
    ▆
 1. └─animint2::animint2pages(viz, github_repo = "animint2pages_test_repo") at test-compiler-ghpages.R:26:3
 2.   └─animint2:::manage_gh_pages(...)
 3.     └─gh::gh(...)
 4.       └─gh:::gh_make_request(req)
 5.         └─gh:::gh_error(resp, gh_req = x, error_call = error_call)
 6.           └─cli::cli_abort(...)
 7.             └─rlang::abort(...)
── Error ('test-compiler-ghpages.R:[53](https://github.com/animint/animint2/actions/runs/9816550931/job/27106887489?pr=138#step:11:54):3'): animint2pages() default branch is gh-pages ──
<GIT_ERROR/libgit2_error/error/condition>
Error in `libgit2::git_clone`: unexpected http status code: 404
Backtrace:
    ▆
 1. ├─gert::git_clone(...) at test-compiler-ghpages.R:53:3
 2. └─gert:::raise_libgit2_error(...)

[ FAIL 2 | WARN 0 | SKIP 0 | PASS 7[59](https://github.com/animint/animint2/actions/runs/9816550931/job/27106887489?pr=138#step:11:60) ]

@ampurr
Copy link
Contributor

ampurr commented Jul 7, 2024 via email

@tdhock
Copy link
Collaborator

tdhock commented Jul 8, 2024

the error does not say anything about expired token.
It says 403 (permission denied) and 404 (not found).

@Faye-yufan
Copy link
Contributor Author

The first failed test:

The first test animint2pages() returns owner/repo string is for pushing a viz repo from local to GitHub remote. I didn't change any code in this test, so it remains the same as in the recent PR #126, where it did not fail (see #126 recent run happened after this PR failed run).

Therefore, the failure in the current GH Actions test is likely due to the newly added code for setting the default branch.

I believe the issue might be caused by the newly added code (https://github.com/animint/animint2/pull/138/files#diff-6c77230e5f2bdcb3a3c21e98224ada8f337bb04f677ede3b90406a3933087509R132-R138), which directly uses the GitHub API gh.

I am trying to use gert instead of gh to see if it resolves the issue.

The second failed test:

It is probably because the first test failed, so no repo can be fetched through gert::git_clone.

@ampurr
Copy link
Contributor

ampurr commented Jul 9, 2024 via email

@Faye-yufan
Copy link
Contributor Author

Faye-yufan commented Jul 9, 2024

The animint2pages() returns owner/repo string test issue, 403 (permission denied), now has been resolved. If user want to create a new viz repo, in the initial commit, it changes the name of master branch to gh-pages, so it is now the default and only branch in the new repo.

The second issue, 404 (not found), remains. Still looking into it.

@Faye-yufan
Copy link
Contributor Author

Faye-yufan commented Jul 9, 2024

I see the test repo is created under @tdhock 's personal GH https://github.com/tdhock/animint2pages_test_repo, because of this setting

- name: git config user.email
run: git config --global user.email toby.hocking@r-project.org

But in above test run, I tried to print the output of whoami[["login"]] in the gh actions enviroment and the log showed

[1] "GitHub Pages"
NULL

Indicates that the gh action is trying to clone "https://github.com/NULL/animint2pages_test_repo.git", which obviously does not exist.

Let me try using credential.

@Faye-yufan
Copy link
Contributor Author

Hi @tdhock , do you recall how you created this test repo https://github.com/tdhock/animint2pages_test_repo ? Was it pushed from your local machine or via a GitHub Actions?
Because I tried to push a new test repo(a2p_ghp_test) under your GH, to test the default branch gh-pages. But now the log shows an error that the env does not have the GitHub token to push to a new repo-

══ Failed tests ════════════════════════════════════════════════════════════════
── Error ('test-compiler-ghpages.R:57:3'): animint2pages() default branch is gh-pages ──
Error in `value[[3L]](cond)`: A GitHub token is required to create and push to a new repository. 
To create a GitHub token, follow these steps:
1. Go to https://github.com/settings/tokens/new?scopes=repo&description=animint2pages
2. Confirm your password if prompted.
3. Ensure that the 'repo' scope is checked.
4. Click 'Generate token' at the bottom of the page.
5. Copy the generated token.
After creating the token, you can set it up in your R environment by running: 
Sys.setenv(GITHUB_PAT="yourGithubPAT") 
gert::git_config_global_set("user.name", "yourUserName") 
gert::git_config_global_set("user.email", "yourEmail") 

Backtrace:
    ▆
 1. └─animint2::animint2pages(viz, github_repo = "a2p_ghp_test") at test-compiler-ghpages.R:[57](https://github.com/animint/animint2/actions/runs/9869578505/job/27253578004#step:11:58):3
 2.   └─base::tryCatch(...)
 3.     └─base (local) tryCatchList(expr, classes, parentenv, handlers)
 4.       └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
 5.         └─value[[3L]](cond)

based on this comment #101 (comment), the animint2pages_test_repo seems has been created before generating the token?

@tdhock
Copy link
Collaborator

tdhock commented Jul 10, 2024

yes that is right, I created the repo using github web interface, then followed those instructions to create PAT which gives animint/animint2 repo permissions to push to tdhock/animint2pages_test_repo during gh actions testing of animint2pages.

@tdhock
Copy link
Collaborator

tdhock commented Jul 10, 2024

Why can't we just keep using animint2pages_test_repo, with the same PAT, and just add a new test to make sure that the default branch that we get when we clone it is gh-pages?

Comment on lines +58 to +61
gert::git_clone(url = paste0("https://github.com/", owner, "/a2p_ghp_test.git"), path = local_repo_path)
# Check the default branch after clone
default_branch <- gert::git_branch(repo = local_repo_path)
expect_equal(default_branch, "gh-pages")
Copy link
Collaborator

Choose a reason for hiding this comment

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

can this test be added inside the "animint2pages() returns owner/repo string" block above?

@Faye-yufan
Copy link
Contributor Author

Why can't we just keep using animint2pages_test_repo, with the same PAT, and just add a new test to make sure that the default branch that we get when we clone it is gh-pages?

Yes we can just use animint2pages_test_repo only. The reason I added a new repo here is just for testing on GH Actions, because animint2pages() function itself cannot change the default branch of an existing GH repo. The test worked on my local, but I wanted to create a new repo with updated animint2pages(), which setsgh-pages as the default branch, to see if the test works on GH Actions.

@Faye-yufan
Copy link
Contributor Author

@tdhock Could you create a new animint2pages_test_repo using this branch version of animint2pages? Or maybe I can try create a test repo under animint org to see if it works.

@tdhock
Copy link
Collaborator

tdhock commented Jul 11, 2024

"Could you create a new animint2pages_test_repo using this branch version of animint2pages?"
I could but I'm not sure that would address this testing issue.
Or maybe I am not understanding your idea, can you please clarify?

the issue is that the current PAT only has write access to one repo, which is already created, but we want to test creation of a new repo, right?
can we create a new org, "animint-testing" and make a PAT that has permission to create new repos in that org?
(I guess that would be safer than making a PAT with repo write access to animint org, or our personal github accounts)

@tdhock
Copy link
Collaborator

tdhock commented Aug 30, 2024

can we create a new org, "animint-testing" and make a PAT that has permission to create new repos in that org?

this was implemented in #147 which also required permission to delete repos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

set default branch of new repo in animint2pages
3 participants