[SERVER-68699] Write a test that checks that resmoke suite config is valid Created: 09/Aug/22  Updated: 27/Dec/23  Resolved: 11/Oct/23

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 7.2.0-rc0

Type: Task Priority: Minor - P4
Reporter: Dave Rolsky Assignee: Mikhail Shchatko
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Problem/Incident
causes SERVER-84428 resmoke no longer errors if given a n... Needs Scheduling
Related
related to SERVER-84426 replica_sets_kill_secondaries_jscore_... Closed
is related to SERVER-81938 Validate tags existence that are conf... Backlog
Assigned Teams:
Server Development Platform
Backwards Compatibility: Fully Compatible
Sprint: DAG 2023-10-16
Participants:
Story Points: 2

 Description   

Recently I noticed that the config for mongosync's 2 fle2 suites had exclude_files entries referring to files that simply no longer existed in the commit of the mongo repo we use. While resmoke does some validation of the contents of these YAML files, there is more it could do.

Acceptance Criteria

  • Resmoke should validate all YAML files that it processes resmokeconfig/suites. For the elements under the selector key, it should do the following checks:
    • For roots, ensure that these paths (with globs expanded) match at least one file.
    • For exclude_files, ensure that these paths (with globs expanded) match at least one file.
    • For exclude_with_any_tags, ensure that this tag exists in at least one test file.
    • Are there any other keys that can be under selector? If so, those should be tested too.


 Comments   
Comment by Githook User [ 11/Oct/23 ]

Author:

{'name': 'Mikhail Shchatko', 'email': 'mikhail.shchatko@mongodb.com', 'username': 'MikhailShchatko'}

Message: SERVER-68699 Check that resmoke suite selector config has valid test paths
Branch: master
https://github.com/mongodb/mongo/commit/c0192285898f774c7682907858c7f22eccd68e7c

Comment by Mikhail Shchatko [ 06/Oct/23 ]

I created SERVER-81938 to address tags validation since I see problems in implementing it right now.
At the moment we have jstests spread across mongodb and enterprise repos. So this check may fail on community variants, because enterprise module is missing there. There is no easy way to collect the information about tags from the enterprise repo on such variants. After we consolidate mongodb and enterprise repos it will become possible to implement.

Comment by Trevor Guidry [ 04/Oct/23 ]

I think you are right, currently resmoke does not have all the context it needs to know what "all tests" is. I think we could add a new config variable similar to our current "config_dir" variable. This new variable would be a list of directories where jstest tests are defined, for us it would default to "jstests" and "src/mongo/db/modules/enterprise/jstests". This new variable would allow us to do two things, recursively search these directories to determine what "all tests" are and only validate suites that use these directories in their selectors. Some suites use non-standard locations and expect certain input. These inputs may or may not be checked in to the git repo and should not be part of this validation.

Example:

new variable,something like jstest_dirs = ["jstests", "src/mongo/db/modules/enterprise/jstests"]

You can scan all tests in these dirs to determine all in use tags

You run into suites like jsCore and validate it because the selector paths start with the above dirs 

You run into suites like generational_fuzzer and do not validate it because it uses non-standard paths

Some caveats I know of

  • For roots, ensure that these paths (with globs expanded) match at least one file.
    • Some suites do not have the files in their selector as part of the git repo by default. Some examples of this are:
    • jstestfuzz tests. They require the jstestfuzzer to run and produce fuzzed files to run. 
    • Maybe some other tests, not sure
  • For exclude_files, ensure that these paths (with globs expanded) match at least one file.
    • This seems pretty straight forward.
  • For exclude_with_any_tags, ensure that this tag exists in at least one test file.
    • In regards to your comment "So the only reasonable solution regarding tags check I see is for example to fail if all configured tags inclusion/exclusion do not affect test selection at all."
      • I have seen suites use tags that do not affect their current selection. For example some encryption suites might be marked "incompatible_with_encryption" but none of the suites in their selector currently are tagged that. In the future it might contain a test tagged that so I think it is still a valid usecase.

 

Resmoke has a lot of edge cases so I might be missing some stuff, hopefully this was clear and helps but feel free to DM me if you have any questions.

Comment by Mikhail Shchatko [ 03/Oct/23 ]

alex.neben@mongodb.com trevor.guidry@mongodb.com Given the conversation above and SERVER-68242 closure as won't do it is not exactly clear if the check for tags existence in at least one file is needed. I assume yes, but still wanted to know what you think.
If above is needed the next question would be how we can achieve this. From various tags configurations we are building ultimate tags_expression here and here. After that we apply it on each test here, so if we have anywhere configured a tag that doesn't affect any test inclusion/exclusion, this seems to me impossible to detect with the current design. Also as far as I know resmoke doesn't know what is "all tests" and can only operate with what suite selector is able to find. So the only reasonable solution regarding tags check I see is for example to fail if all configured tags inclusion/exclusion do not affect test selection at all.
Please, let me know if that makes sense and what you think.

Comment by Robert Guo (Inactive) [ 11/Aug/22 ]

Got it. Validating roots makes sense. If you'd like, feel free to add a PR with an additional assert here.

Regarding maintaining the tags, there would be pre-commit enforcement to ensure tags in files are kept up-to-date. It should be doable to backport to older commits.

Comment by Dave Rolsky [ 10/Aug/22 ]

robert.guo@mongodb.com I'd also note that the other part of this ticket is to check that roots is valid. I think that would still be useful.

Comment by Dave Rolsky [ 10/Aug/22 ]

robert.guo@mongodb.com, this sounds like a good direction to go. My only question would be how other teams would handle cases where we need to exclude something. How can we get a tag added to a test if it doesn't have what we need? This is further complicated by the fact that right now we are locked into a single older commit of the Server code which we check out to run the resmoke tests with. I'm not sure how hard it is for us to change that commit, though.

Comment by Robert Guo (Inactive) [ 09/Aug/22 ]

Hey dave.rolsky@mongodb.com thanks for raising this issue. The interface between mongosync and the server test driver has been brittle and a frequent source of bugs. There was a discussion earlier today about making JS tests more knowable (as part of our dev prod 6 week check in) and we plan to try standardizing tags as a way to make testing more knowable. The work is in SERVER-68242 and scheduled for the current quarter. Better categorization of tags will allow resmoke to extricate itself from being the "source of truth" of test coverage, which is a fairly complex problem space.

The adoption plan after that is to have suites only exclude tests using tags. Do you think this would be a reasonable solution to your pain point? The other side of the story is that the proposed change here is likely fairly involved as resmoke has to support many types of tests and environments and many such environments (e.g. ones with automated test selection), will have some tags being empty.

Generated at Thu Feb 08 06:11:30 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.