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

Update integration test pack versions #721

Merged
merged 2 commits into from
Sep 1, 2021

Conversation

aeisenberg
Copy link
Contributor

@aeisenberg aeisenberg commented Aug 26, 2021

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

@aeisenberg aeisenberg requested a review from a team as a code owner August 26, 2021 22:59
Copy link
Contributor

@adityasharad adityasharad left a comment

Choose a reason for hiding this comment

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

LGTM if tests pass.
Should these always use the latest version? Or is the point that we're testing both explicit versions and implicit latest versions?

@aeisenberg
Copy link
Contributor Author

Darn...tests still failing. I think I need to create a version 0.0.4 of javascript-all. There must be some dbscheme change that hasn't yet been picked up.

@aeisenberg
Copy link
Contributor Author

It's still failing. I'm not sure exactly why. I updated both test packs to use the latest javascript-all pack. AFAICT the last change to the semmlecode.javascript.dbscheme file was when I migrated to the lib folder. This change was not in the 2.6.0 release, which the integration tests are currently using, so I still need to investigate why we are getting this error.

@adityasharad
Copy link
Contributor

test-split-workflow is using 2.6.0 and the other three failing tests test-packaging-javascript* are using 2.5.9 (presumably the version in the toolcache). I'm not sure that's the main problem, but as a starting point, perhaps try those tests with tools: latest to force them to use 2.6.0?

@aeisenberg
Copy link
Contributor Author

I guess it's also possible that the dbscheme of the compiled queries are too new and the upgrade script is not available, so the queries are failing. I'll have to look at this next week.

@aeisenberg aeisenberg force-pushed the aeisenberg/update-pack-version branch from aab4833 to a9da277 Compare August 27, 2021 22:26
@aeisenberg
Copy link
Contributor Author

I forced the failing tests to use the nightly build. Now they are passing.

Copy link
Contributor

@edoardopirovano edoardopirovano left a comment

Choose a reason for hiding this comment

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

As a very temporary fix to unbreak the tests this seems okay, but we definitely need a more permanent solution that doesn't require us to keep manually bumping versions.

@aeisenberg
Copy link
Contributor Author

I forced the failing tests to use the nightly build. Now they are passing.

In retrospect, this is misleading. I wasn't implying that this is a good solution, or even something I'd like to commit. I'm planning on working on this tomorrow and finding out what is happening, because upgrades should work.

@edoardopirovano as you said, this would only be temporary. If you are being blocked by failing builds, then it is ok to merge. But, I would prefer to explore deeper when I am able to.

@edoardopirovano
Copy link
Contributor

Agree that we should investigate why the upgrades aren't working. I'm not blocked on failing tests, so okay to not merge this and instead wait until we have a proper solution.

@aeisenberg aeisenberg force-pushed the aeisenberg/update-pack-version branch from fb4dd05 to a033359 Compare August 30, 2021 20:15
@aeisenberg
Copy link
Contributor Author

The 0.1.0 versions fail because the queries are too new and upgrades are not found. The bug is that we haven't updated codeql resolve upgrades to work with packs yet.

I'll work on this next.

@aeisenberg
Copy link
Contributor Author

These tests will not be properly fixed until the next release of the CLI. This is because database upgrades is not yet package-aware. I'll fix that shortly, but it won't be available until the release.

@aeisenberg aeisenberg closed this Aug 31, 2021
@aeisenberg aeisenberg reopened this Aug 31, 2021
@aeisenberg
Copy link
Contributor Author

Building a new nightly version of the CLI. I expect that once that version is available to the action, then this PR will pass and we can get the action back on track.

@aeisenberg aeisenberg force-pushed the aeisenberg/update-pack-version branch from a033359 to 8782c97 Compare August 31, 2021 22:35
@aeisenberg aeisenberg force-pushed the aeisenberg/update-pack-version branch from 8782c97 to d1ff4d6 Compare September 1, 2021 04:34
@aeisenberg aeisenberg enabled auto-merge September 1, 2021 04:38
@aeisenberg
Copy link
Contributor Author

This latest version hard-codes the tools bundle being used by the packaging integration tests. This will work for now. However, we need to make sure that the version that is pointed to is not cleaned up by automation.

@aeisenberg
Copy link
Contributor Author

I made sure that the release is not cleaned up by automation by making it a non-prerelease. Now, it will always show on top of the releases list, which always keeps the 10 latest.

@aeisenberg aeisenberg merged commit c0a5878 into main Sep 1, 2021
@aeisenberg aeisenberg deleted the aeisenberg/update-pack-version branch September 1, 2021 04:47
@github-actions github-actions bot mentioned this pull request Sep 6, 2021
5 tasks
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.

3 participants