Skip to content

Add starter kit to user tests #19934

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

Merged
merged 5 commits into from
Nov 13, 2017
Merged

Add starter kit to user tests #19934

merged 5 commits into from
Nov 13, 2017

Conversation

sandersn
Copy link
Member

Add starter kit repos to user tests. This works by looking for a test.json file in the test directory. If one is present, then the runner updates submodules, the changes the working directory to the submodule's subdirectory.

The types field from test.json is used to prevent errors from compiling inside of the Typescript project. And the --noEmit flag is added to everything to allow empty --types to work, in addition to speeding up test times.

If there is a test.json in the directory, it expects to find a
submodule in the directory. The submodule should have the same name as
the directory itself. test.json contains a list of global types that
need to be available, or the empty list if none.
@sandersn sandersn requested a review from weswigham November 10, 2017 23:38
@sandersn
Copy link
Member Author

I did not baseline React, React-Native or WeChat, even though they currently fail, because I have a simple change to tsconfig for each.

Even if we need to fully upgrade each repo to the latest versions of their respective libraries, I think we can do that instead fairly quickly.

if (update.status !== 0) throw new Error(`git submodule update for ${directoryName} failed!`);

const config = JSON.parse(fs.readFileSync(path.join(cwd, "test.json"), { encoding: "utf8" })) as UserConfig;
ts.Debug.assert(!!config.types, "Git is the only reason for using test.json right now");
Copy link
Member

Choose a reason for hiding this comment

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

"Git is the only reason for using test.json right now"? That's not what's being checked here, so it's odd. I think "Types field must be present" would be sufficient. And likely not need to change in the future.

Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

Seems good.

Inconsequential question:
The tests update the submodules regularly, which is good; do we want a bot to commit those updated submodules when they change without consequence to the baselines? I think they'll show up in git status (not sure) each time they're changed, if that affects your response.

@sandersn
Copy link
Member Author

That's an interesting question that we should discuss in person. I'm not sure which you are suggesting:

  1. I don't expect people to run this locally except when it fails. This is a good time to commit changes since (1) either the test needs to be updated or (2) something is wrong with Typescript itself. Either way, we will be committing changes.
  2. Since cron is running this daily, it could commit submodule updates if the daily run passes.

(1) seems more desirable to me, and easier to write, since (I guess) it's just an additional call to git add after updating the submodule. And we don't have to worry about a cron job going mad with power and committing 1,000,000 erroneous/insane commits.

@sandersn sandersn merged commit 7771d0c into master Nov 13, 2017
@sandersn sandersn deleted the add-starter-kit-to-user-tests branch November 13, 2017 16:54
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants