Skip to content

Add expected files lists#1101

Open
misolt wants to merge 1 commit intogoogle:mainfrom
misolt:expected-files
Open

Add expected files lists#1101
misolt wants to merge 1 commit intogoogle:mainfrom
misolt:expected-files

Conversation

@misolt
Copy link
Copy Markdown
Member

@misolt misolt commented Mar 13, 2026

Manual merge of krykania#10

@misolt misolt marked this pull request as ready for review March 13, 2026 16:00
@@ -0,0 +1,14 @@
{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I do not see that these files are part of source code. instead these files looks like tests configuration and it will be better to keep these files closer to the place they will be used (tests).

Copy link
Copy Markdown

@jakubwrobel00 jakubwrobel00 Mar 18, 2026

Choose a reason for hiding this comment

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

Our initial idea was that introducing the files list configuration in the dumper source code would make life easier for the PR owner, for example when the PR replaces a file name with a new one, the light tests will fail, as the file from the file list is not found inside the dump. Then the developer could update the file list with the new file name and the tests would succeed. If we place the file lists in the tests repository, one would have to make another CL to update the files list and this could be annoying.

These are of course only our thoughts, and if you don't think the files belong in the dumper repository, we will move them, just let us know.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The idea is nice.

My concern is these files still not part of the source code and if I as developer will do a typo or I will just forget to update the list everything will work for me. It's why I think this config should not be here.

I see at least 2 better options:

  1. Move files to internal google repo (where the integration tests are located)
  2. create some configuration folder in the root of the repo GitHub and move these files to this folder. (examples .github/ , .security/)

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