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

routes/crate: Fetch crate when peeked crate lacks included values #10666

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

eth3lbert
Copy link
Contributor

Fixes #10663 .

@eth3lbert eth3lbert added C-bug 🐞 Category: unintended, undesired behavior A-frontend 🐹 labels Feb 23, 2025
@eth3lbert eth3lbert force-pushed the fix-missing-keywords branch from 2330c30 to 0b512ce Compare February 23, 2025 15:16
Comment on lines +249 to +257
test('keywords are shown when navigating from search', async ({ page, msw }) => {
loadFixtures(msw.db);

await page.goto('/search?q=nanomsg');
await page.getByRole('link', { name: 'nanomsg', exact: true }).click();

await expect(page).toHaveURL('/crates/nanomsg');
await expect(page.locator('[data-test-keyword]')).toBeVisible();
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's another case that would cause the same issue: navigating from dependencies that have more than one dependency (as this will query with ?ids=). We would need to expand the fixtures to have at least two dependencies. Let me know if we'd like to include this as well.

Copy link
Member

Choose a reason for hiding this comment

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

probably fine as-is. if anything, IMHO we shouldn't extend the hardcoded fixtures and instead setup a dedicated scenario for the test. I would prefer to get rid of the fixtures in general.

@eth3lbert eth3lbert requested a review from Turbo87 February 23, 2025 15:31
@eth3lbert
Copy link
Contributor Author

I re-ran the failed tests, and those failures seem unrelated to this PR.

@Turbo87 Turbo87 force-pushed the fix-missing-keywords branch from 0b512ce to 98ae017 Compare February 24, 2025 10:15
@Turbo87 Turbo87 enabled auto-merge (squash) February 24, 2025 10:16
@Turbo87 Turbo87 merged commit b4c801d into rust-lang:main Feb 24, 2025
10 of 11 checks passed
@eth3lbert eth3lbert deleted the fix-missing-keywords branch February 24, 2025 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-frontend 🐹 C-bug 🐞 Category: unintended, undesired behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keywords are sometimes missing
2 participants