feat: blacklist -> ignorelist and platform fixes#57
Closed
vladimirvolek wants to merge 6 commits intomainfrom
Closed
feat: blacklist -> ignorelist and platform fixes#57vladimirvolek wants to merge 6 commits intomainfrom
vladimirvolek wants to merge 6 commits intomainfrom
Conversation
vladimirvolek
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the test-generation/fixture system to replace “blacklist” terminology with “ignorelist”, adds an IGNORELIST_ONLY mode for CI workflows, and updates address transaction fixtures to better separate paginated vs from/to query behaviors.
Changes:
- Rename blacklist rules/utilities to ignorelist equivalents and load ignorelist entries by network.
- Add
IGNORELIST_ONLYmode to generate only ignored tests, plus improved skipped-test reporting. - Update preprod/mainnet
address-transactionsfixtures (IDs/names and precached responses) to reflect paginated vs from/to cases.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
unit/index.test.ts |
Renames unit tests to use matchesIgnoreRule. |
src/index.ts |
Implements ignorelist loading + IGNORELIST_ONLY gating and changes skipped-test logging. |
src/config.ts |
Adds IGNORELIST_ONLY config and enforces NETWORK presence. |
src/fixtures/preprod/addresses/address-transactions.ts |
Splits paginated vs from/to fixture cases and adds explicit responses. |
src/fixtures/mainnet/addresses/address-transactions.ts |
Adds shared precached response data and renames/splits fixtures for paginated vs from/to variants. |
Comments suppressed due to low confidence (1)
src/index.ts:29
envConfigis initialized at module load time viagetConfig(). With the newNETWORKrequired check, importing this module will throw even for callers that only need pure functions (e.g.isUrlMatch). To avoid import-time crashes, consider deferringgetConfig()(and ignorelist loading) untilgenerateTestSuite/ network-dependent code paths are invoked.
const __dirname = path.dirname(fileURLToPath(import.meta.url));
const filePath = path.resolve(__dirname, '../endpoints-allowlist.json');
const ignorelistFilePath = path.resolve(__dirname, '../endpoints-ignorelist.json');
export const DEFAULT_TEST_TIMEOUT = 15_000;
const envConfig = getConfig();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+199
to
+201
| const ignored = skippedTests.filter(t => t.reason.startsWith('On the ignorelist')); | ||
| const notInAllowlist = skippedTests.filter(t => !t.reason.startsWith('On the ignorelist')); | ||
|
|
Comment on lines
+219
to
+223
| lines.push(` Not in allowlist (${notInAllowlist.length}):`); | ||
|
|
||
| for (const t of notInAllowlist) { | ||
| lines.push(` ⊘ [${t.id}] ${t.testName} — ${t.endpoint}`); | ||
| } |
Comment on lines
+58
to
+80
| const loadIgnorelist = (filePath: string): IgnoreRule[] => { | ||
| if (!fs.existsSync(filePath)) { | ||
| return []; | ||
| } | ||
|
|
||
| if (fs.existsSync(blacklistFilePath)) { | ||
| try { | ||
| const rawData = fs.readFileSync(blacklistFilePath, 'utf8'); | ||
| const network = envConfig.network; | ||
|
|
||
| try { | ||
| const rawData = fs.readFileSync(filePath, 'utf8'); | ||
| const parsed: unknown = JSON.parse(rawData); | ||
|
|
||
| if (!Array.isArray(parsed)) { | ||
| throw new Error('Expected endpoints-blacklist.json to contain a JSON array.'); | ||
| if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { | ||
| throw new Error(`Expected ${path.basename(filePath)} to be a JSON object keyed by network.`); | ||
| } | ||
|
|
||
| const entries = (parsed as Record<string, IgnoreRule[]>)[network]; | ||
|
|
||
| if (!entries) { | ||
| return []; | ||
| } | ||
|
|
||
| if (!Array.isArray(entries)) { | ||
| throw new Error(`Expected ${path.basename(filePath)}["${network}"] to be an array.`); |
Comment on lines
+12
to
15
| if (!network) { | ||
| throw new Error(`NETWORK env is not set.`); | ||
| } | ||
|
|
Comment on lines
1
to
3
| import { describe, expect, it } from 'vitest'; | ||
| import { isUrlMatch, matchesBlacklistRule } from '../src/index.js'; | ||
| import { isUrlMatch, matchesIgnoreRule } from '../src/index.js'; | ||
| import { Fixture } from '../src/types/index.js'; |
|
|
||
| if (!blacklisted && shouldRunTest(fixture)) { | ||
| // In IGNORELIST_ONLY mode, only run tests that are on the ignorelist. | ||
| // This is used by CI to verify blacklisted tests still fail. |
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.