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

Integration tests are not always pulling head commit #1993

Closed
davidcassany opened this issue Mar 6, 2024 · 2 comments
Closed

Integration tests are not always pulling head commit #1993

davidcassany opened this issue Mar 6, 2024 · 2 comments

Comments

@davidcassany
Copy link
Contributor

davidcassany commented Mar 6, 2024

I can't really find what is going on but I got a couple of clear examples that integration tests are not running the expected code. Most obvious is the difference from these two runs from PR #1992:

  1. https://github.com/rancher/elemental-toolkit/actions/runs/8175584404
  2. https://github.com/rancher/elemental-toolkit/actions/runs/8176020420

The first run and the second run they only diverge by this empty commit 406899a on first run on workflow logs we can see the extra argument in makefile was not added in actual worlflow run, however we can see it in the second. In both cases the code was exactly the same.

I have see this issue more than once lately, but now I managed to collect an example that clearly shows the issue.

And the very same thing happened again on the same PR. Added a fix for recovery test

  1. https://github.com/rancher/elemental-toolkit/actions/runs/8176968369 in this run we can see the recovery test does not run the modified command
  2. https://github.com/rancher/elemental-toolkit/actions/runs/8177628804 shows that after another empty commit now runs the modified command as it was expected from the previous run.

Feels like if we were always testing one commit behind... so weird 🤷🏽‍♂️ This already puzzled me several times in the past few weeks...

@davidcassany davidcassany moved this to 🗳️ To Do in Elemental Mar 6, 2024
@davidcassany
Copy link
Contributor Author

davidcassany commented Mar 7, 2024

Just found another example https://github.com/rancher/elemental-toolkit/actions/runs/8186424112?pr=1995

There is a cache hit, but the PR actually changes the code. Hence the cache hit is not possible as this hash includes tracking changes under /pkg, hence my guess is that this is just using a different code.

In fact in that same run if we check the toolkit commit is actually building this is the HEAD of main branch, hence the toolkit built with this tests is not including the actual changes.

Interesting enough on build-toolkit job I see the checking out the ref step for checkout/v4 action I see this

  /usr/bin/git checkout --progress --force -B main refs/remotes/origin/main
  Switched to a new branch 'main'
  branch 'main' set up to track 'origin/main'.
/usr/bin/git log -1 --format='%H'
'c03dc357e65c962a0bda47f97a18108ee07a207f'

Where c03dc37 is the commit of main.

However after adding an empty commit in the same PR and in the same step I see:

Note: switching to '63ebec7b86b255152c57d06660e5ecf044fcea01'.
  
  You are in 'detached HEAD' state. You can look around, make experimental
  changes and commit them, and you can discard any commits you make in this
  state without impacting any branches by switching back to a branch.
  
  If you want to create a new branch to retain commits you create, you may
  do so (now or later) by using -c with the switch command. Example:
  
    git switch -c <new-branch-name>
  
  Or undo this operation with:
  
    git switch -
  
  Turn off this advice by setting config variable advice.detachedHead to false
  
  HEAD is now at 63ebec7 Merge b6095542a14018cffe2e56919b460c4daced207e into c03dc357e65c962a0bda47f97a18108ee07a207f

Which clearly points to the fact the code is the merge to main so it tests the code including the changes of the PR. 😕

Then again b609554 commit, is not the latest of #1995 at that time, it is just the previous one. It really feels it always checkouts HEAD~1 for the given PR.

@frelon
Copy link
Contributor

frelon commented Mar 8, 2024

solved in #1996, verified in #1997:

HEAD is now at 7f0b24b Merge 11806b9ef1d3ea821a2a92f2e12522c516916680 into d522f6f29ddd0cc9915bff89f7cb3b8a809e6782

@frelon frelon closed this as completed Mar 8, 2024
@github-project-automation github-project-automation bot moved this from 🗳️ To Do to ✅ Done in Elemental Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

2 participants