Add integration tests for project with complex package version requirements #2792
Conversation
MDrakos
commented
Oct 4, 2023
•
MDrakos
commented
 DX-2241 UNINSTALL: ALL ST VERSIONS install/uninstall packages fail due build
|
|
Failing Windows integration tests are not related to this PR change. |
There was a problem hiding this comment.
This looks good, but my one gripe is I can't tell from the test itself what's being tested.
I'd recommend either adding comments or updating the tests themselves so it's clear what is being tested for.
Point being I don't know what I would do if this test started failing and/or needed updates.
|
I tried adding an import step with the requirements data that I used to set up the project but I kept running into timeout issues due to the project building. Instead, I've added a comment that hopefully helps understand what the test is doing. We could still run into timeout issues but it doesn't appear to be happening with the current project and test configuration. The other windows failures are unrelated to the PR. |
|
What we want to test is that we can make modifications to a build expression that has complex version requirements, submit that, and get the expected response. The test I added mirrors how the bug surfaced. That said, the test you propose is also something worth having so I've added it. Note that for the new test, I'm not verifying the version numbers because of the bug I filed earlier today: https://activestatef.atlassian.net/browse/DX-2272 |
| cp = ts.Spawn("packages") | ||
| cp.Expect("Mopidy-Dirble") | ||
| cp.Expect("coverage") | ||
| cp.Expect("docopt") | ||
| cp.Expect("requests") | ||
| cp.Expect("urllib3") |
There was a problem hiding this comment.
Since you can't use state packages and since verifying the version numbers is the point of this tests maybe instead we can use state history?
I don't see any point in adding the test as is because again it is not testing what it is set out to test.
| func (suite *PackageIntegrationTestSuite) TestProjectWithCustomVersionRequirements() { | ||
| suite.OnlyRunForTags(tagsuite.Package) | ||
| if runtime.GOOS == "windows" { | ||
| suite.T().Skip("Skipping windows as builds can be slow and we only really need to test this one one platform") | ||
| return | ||
| } | ||
| ts := e2e.New(suite.T(), false) | ||
| defer ts.Close() | ||
|
|
||
| // Because the ActiveState-CLI org has enterprise features enabled, we need to login | ||
| ts.LoginAsPersistentUser() | ||
|
|
||
| // This project was created with the following requirements.txt: | ||
| // coverage!=3.5 | ||
| // docopt>=0.6.1 | ||
| // Mopidy-Dirble>=1.1,<2 | ||
| // requests>=2.2,<2.31.0 | ||
| // urllib3>=1.21.1,<=1.26.5 | ||
| cp := ts.Spawn("checkout", "ActiveState-CLI/Comparators", ".") | ||
| cp.Expect("Skipping runtime setup") | ||
| cp.Expect("Checked out project") | ||
| cp.ExpectExitCode(0) | ||
|
|
||
| // Uninstall all of the packages with custom version requirements | ||
| cp = ts.Spawn("uninstall", "coverage") | ||
| cp.ExpectExitCode(0) | ||
|
|
||
| cp = ts.Spawn("uninstall", "docopt") | ||
| cp.ExpectExitCode(0) | ||
|
|
||
| cp = ts.Spawn("uninstall", "Mopidy-Dirble") | ||
| cp.ExpectExitCode(0) | ||
|
|
||
| cp = ts.Spawn("uninstall", "requests") | ||
| cp.ExpectExitCode(0) | ||
|
|
||
| cp = ts.Spawn("uninstall", "urllib3") | ||
| cp.ExpectExitCode(0) | ||
| } |
There was a problem hiding this comment.
With your latest explanation I think I am caught up. This test should not exist. We are testing an API now, rather than the state tool. That's not our job, we have a hard enough time keeping our tests under control with just covering the State Tool itself.
The new test you added has value but does seem unrelated to the story at hand. Perhaps we should abandon the ticket and file a new one for the import test.
|
Closing in favor of: https://activestatef.atlassian.net/browse/DX-2276 |
