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

Use a more stable method to detect rosdep changes #30694

Merged
merged 2 commits into from
Sep 8, 2021

Conversation

cottsay
Copy link
Member

@cottsay cottsay commented Sep 3, 2021

The method for detecting rosdep changes in the repository checking system doesn't seem to work very well under all circumstances, and sometimes fails to determine the differences resulting in a failed job.

This change switches the test to use a line change detection mechanism already in use in another test, so it should be more stable.

Based on:

cmd = ('git config --get remote.%s.url' % UPSTREAM_NAME).split()
try:
remote_url = subprocess.check_output(cmd).decode('utf-8').strip()
# Remote exists
# Check url
if remote_url != DIFF_REPO:
detected_errors.append('%s remote url [%s] is different than %s' % (UPSTREAM_NAME, remote_url, DIFF_REPO))
return detected_errors
target_branch = '%s/%s' % (UPSTREAM_NAME, DIFF_BRANCH)
except subprocess.CalledProcessError:
# No remote so fall back to origin/master
print('WARNING: No remote %s detected, falling back to origin master. Make sure it is up to date.' % UPSTREAM_NAME)
target_branch = 'origin/master'
cmd = ('git diff --unified=0 %s' % target_branch).split()
diff = subprocess.check_output(cmd).decode('utf-8')

Reverts #30032
Should fix #29964

The method for detecting rosdep changes in the repository checking
system doesn't seem to work very well under all circumstances, and
sometimes fails to determine the differences resulting in a failed job.

This change switches the test to use a line change detection mechanism
already in use in another test, so it should be more stable.

Reverts bdf1c62
@cottsay cottsay self-assigned this Sep 3, 2021
Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM

@ivanpauno
Copy link
Contributor

Thanks for this @cottsay!

I see now failures that are reported in the wrong line, e.g. https://github.com/ros/rosdistro/pull/30565/files.
Is that related to this PR?

Maybe, there's a way to detect the changed lines comparing with master and then look up for the key names in the user branch to get the line numbers correctly (?).

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.

CI failure: "fatal: remotes/origin/master...HEAD: no merge base"
4 participants