Skip to content

Commit

Permalink
Fix overzealous globbing (#2185)
Browse files Browse the repository at this point in the history
* don’t set new project if run mode

* only glob for spec file up to 3 levels deep

* add more debug logs

* add e2e test for spec globbing

* fixes failing tests, add unit test around not setting isNewProject

* downgrade back to 0.2.0
  • Loading branch information
chrisbreiding authored and brian-mann committed Jul 23, 2018
1 parent 3dc4ff9 commit bf70c90
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 30 deletions.
2 changes: 2 additions & 0 deletions packages/server/lib/browsers/electron.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,10 @@ module.exports = {
open: (browserName, url, options = {}, automation) ->
{ projectRoot, isTextTerminal } = options

debug("open %o", { browserName, url })
savedState(projectRoot, isTextTerminal)
.then (state) ->
debug("got saved state")
state.get()
.then (state) =>
debug("received saved state %o", state)
Expand Down
41 changes: 21 additions & 20 deletions packages/server/lib/config.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ origin = require("./util/origin")
coerce = require("./util/coerce")
settings = require("./util/settings")
v = require("./util/validation")
log = require("debug")("cypress:server:config")
debug = require("debug")("cypress:server:config")
pathHelpers = require("./util/path_helpers")

## cypress followed by _
Expand Down Expand Up @@ -361,8 +361,10 @@ module.exports = {
obj.integrationExampleName = scaffold.integrationExampleName()
obj.integrationExamplePath = path.join(obj.integrationFolder, obj.integrationExampleName)

debug("set scaffold paths")
scaffold.fileTree(obj)
.then (fileTree) ->
debug("got file tree")
obj.scaffoldedFiles = fileTree

return obj
Expand All @@ -375,17 +377,16 @@ module.exports = {

## TODO move this logic to find support file into util/path_helpers
sf = obj.supportFile
log "setting support file #{sf}"
log "for project root #{obj.projectRoot}"
debug("setting support file #{sf}")
debug("for project root #{obj.projectRoot}")

Promise
.try ->
## resolve full path with extension
obj.supportFile = require.resolve(sf)
.then ->
if pathHelpers.checkIfResolveChangedRootFolder(obj.supportFile, sf)
log("require.resolve switched support folder from %s to %s",
sf, obj.supportFile)
debug("require.resolve switched support folder from %s to %s", sf, obj.supportFile)
# this means the path was probably symlinked, like
# /tmp/foo -> /private/tmp/foo
# which can confuse the rest of the code
Expand All @@ -395,33 +396,33 @@ module.exports = {
.then (found) ->
if not found
errors.throw("SUPPORT_FILE_NOT_FOUND", obj.supportFile)
log("switching to found file %s", obj.supportFile)
debug("switching to found file %s", obj.supportFile)
.catch({code: "MODULE_NOT_FOUND"}, ->
log("support file %s does not exist", sf)
debug("support file %s does not exist", sf)
## supportFile doesn't exist on disk
if sf is path.resolve(obj.projectRoot, defaults.supportFile)
log("support file is default, check if #{path.dirname(sf)} exists")
debug("support file is default, check if #{path.dirname(sf)} exists")
return fs.pathExists(sf)
.then (found) ->
if found
log("support folder exists, set supportFile to false")
debug("support folder exists, set supportFile to false")
## if the directory exists, set it to false so it's ignored
obj.supportFile = false
else
log("support folder does not exist, set to default index.js")
debug("support folder does not exist, set to default index.js")
## otherwise, set it up to be scaffolded later
obj.supportFile = path.join(sf, "index.js")
return obj
else
log("support file is not default")
debug("support file is not default")
## they have it explicitly set, so it should be there
errors.throw("SUPPORT_FILE_NOT_FOUND", path.resolve(obj.projectRoot, sf))
)
.then ->
if obj.supportFile
## set config.supportFolder to its directory
obj.supportFolder = path.dirname(obj.supportFile)
log "set support folder #{obj.supportFolder}"
debug("set support folder #{obj.supportFolder}")
obj

## set pluginsFile to an absolute path with the following rules:
Expand All @@ -445,31 +446,31 @@ module.exports = {

pluginsFile = obj.pluginsFile

log("setting plugins file #{pluginsFile}")
log("for project root #{obj.projectRoot}")
debug("setting plugins file #{pluginsFile}")
debug("for project root #{obj.projectRoot}")

Promise
.try ->
## resolve full path with extension
obj.pluginsFile = require.resolve(pluginsFile)
log("set pluginsFile to #{obj.pluginsFile}")
debug("set pluginsFile to #{obj.pluginsFile}")
.catch {code: "MODULE_NOT_FOUND"}, ->
log("plugins file does not exist")
debug("plugins file does not exist")
if pluginsFile is path.resolve(obj.projectRoot, defaults.pluginsFile)
log("plugins file is default, check if #{path.dirname(pluginsFile)} exists")
debug("plugins file is default, check if #{path.dirname(pluginsFile)} exists")
fs.pathExists(pluginsFile)
.then (found) ->
if found
log("plugins folder exists, set pluginsFile to false")
debug("plugins folder exists, set pluginsFile to false")
## if the directory exists, set it to false so it's ignored
obj.pluginsFile = false
else
log("plugins folder does not exist, set to default index.js")
debug("plugins folder does not exist, set to default index.js")
## otherwise, set it up to be scaffolded later
obj.pluginsFile = path.join(pluginsFile, "index.js")
return obj
else
log("plugins file is not default")
debug("plugins file is not default")
## they have it explicitly set, so it should be there
errors.throw("PLUGINS_FILE_ERROR", path.resolve(obj.projectRoot, pluginsFile))
.return(obj)
Expand Down
16 changes: 9 additions & 7 deletions packages/server/lib/project.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Promise = require("bluebird")
commitInfo = require("@cypress/commit-info")
la = require("lazy-ass")
check = require("check-more-types")
scaffoldLog = require("debug")("cypress:server:scaffold")
scaffoldDebug = require("debug")("cypress:server:scaffold")
debug = require("debug")("cypress:server:project")
cwd = require("./cwd")
api = require("./api")
Expand Down Expand Up @@ -310,7 +310,12 @@ class Project extends EE
## with additional object "state" which are transient things like
## window width and height, DevTools open or not, etc.
getConfig: (options = {}) =>
if @cfg
return Promise.resolve(@cfg)

setNewProject = (cfg) =>
return if cfg.isTextTerminal

## decide if new project by asking scaffold
## and looking at previously saved user state
if not cfg.integrationFolder
Expand All @@ -319,16 +324,12 @@ class Project extends EE
@determineIsNewProject(cfg.integrationFolder)
.then (untouchedScaffold) ->
userHasSeenOnBoarding = _.get(cfg, 'state.showedOnBoardingModal', false)
scaffoldLog("untouched scaffold #{untouchedScaffold} modal closed #{userHasSeenOnBoarding}")
scaffoldDebug("untouched scaffold #{untouchedScaffold} modal closed #{userHasSeenOnBoarding}")
cfg.isNewProject = untouchedScaffold && !userHasSeenOnBoarding
.return(cfg)

if c = @cfg
return Promise.resolve(c)

config.get(@projectRoot, options)
.then (cfg) => @_setSavedState(cfg)
.then(setNewProject)
.tap(setNewProject)

# forces saving of project's state by first merging with argument
saveState: (stateChanges = {}) ->
Expand All @@ -343,6 +344,7 @@ class Project extends EE
newState

_setSavedState: (cfg) ->
debug("get saved state")
savedState(@projectRoot, cfg.isTextTerminal)
.then (state) -> state.get()
.then (state) ->
Expand Down
4 changes: 2 additions & 2 deletions packages/server/lib/saved_state.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ normalizeAndWhitelistSet = (set, key, value) ->

set(_.pick(valueObject, whitelist))

findSavedSate = (projectRoot, isTextTerminal) ->
findSavedState = (projectRoot, isTextTerminal) ->
if isTextTerminal
debug("noop saved state")
return Promise.resolve(FileUtil.noopFile)
Expand All @@ -58,4 +58,4 @@ findSavedSate = (projectRoot, isTextTerminal) ->
stateFiles[fullStatePath] = stateFile
stateFile

module.exports = findSavedSate
module.exports = findSavedState
4 changes: 3 additions & 1 deletion packages/server/lib/scaffold.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ isNewProject = (integrationFolder) ->
## 3. the files are named the same as the example files
## 4. the bytes of the files match the example files

glob("**/*", { cwd: integrationFolder, realpath: true, nodir: true })
debug("determine if new project by globbing files in %o", { integrationFolder })
## checks for file up to 3 levels deep
glob("{*,*/*,*/*/*}", { cwd: integrationFolder, realpath: true, nodir: true })
.then (files) ->
debug("found #{files.length} files in folder #{integrationFolder}")
debug("determine if we should scaffold:")
Expand Down
2 changes: 2 additions & 0 deletions packages/server/test/support/helpers/e2e.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ module.exports = {
if spec = options.spec
## normalize into array and then prefix
specs = spec.split(',').map (spec) ->
return spec if path.isAbsolute(spec)

path.join(options.project, "cypress", "integration", spec)

## normalize the path to the spec
Expand Down
11 changes: 11 additions & 0 deletions packages/server/test/unit/project_spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,17 @@ describe "lib/project", ->
}
})

it "does not set cfg.isNewProject when cfg.isTextTerminal", ->
cfg = { isTextTerminal: true }

config.get.resolves(cfg)

sinon.stub(@project, "_setSavedState").resolves(cfg)

@project.getConfig({foo: "bar"})
.then (cfg) ->
expect(cfg).not.to.have.property("isNewProject")

context "#open", ->
beforeEach ->
sinon.stub(@project, "watchSettingsAndStartWebsockets").resolves()
Expand Down

0 comments on commit bf70c90

Please sign in to comment.