Skip to content

File Config is no longer a requirement for remoteappmanager#170

Merged
stefanoborini merged 8 commits into
masterfrom
enh-default-do-not-require-remoteappmanager-file-config
Jul 27, 2016
Merged

File Config is no longer a requirement for remoteappmanager#170
stefanoborini merged 8 commits into
masterfrom
enh-default-do-not-require-remoteappmanager-file-config

Conversation

@kitchoi

@kitchoi kitchoi commented Jul 26, 2016

Copy link
Copy Markdown
Contributor

Fix #138

@kitchoi

kitchoi commented Jul 26, 2016

Copy link
Copy Markdown
Contributor Author

Need to update the doc for this change.

@codecov-io

codecov-io commented Jul 26, 2016

Copy link
Copy Markdown

Current coverage is 86.32% (diff: 100%)

Merging #170 into master will increase coverage by 1.18%

@@             master       #170   diff @@
==========================================
  Files            36         35     -1   
  Lines          1379       1360    -19   
  Methods           0          0          
  Messages          0          0          
  Branches        130        131     +1   
==========================================
  Hits           1174       1174          
+ Misses          166        148    -18   
+ Partials         39         38     -1   

Powered by Codecov. Last update 728f540...7755b28

def test_args_without_config_file_path(self):
args = self.spawner.get_args()
self.assertIn("--proxy-api-url=http://127.0.0.1:12345/foo/bar/", args)
self.assertFalse(any("--config-file=" in arg for arg in args))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

or you could "config-file" in " ".join(args)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, but I think that loops over the content ofargs twice? any returns True for the first True (although if the test should pass, it loops over the whole thing, once).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you seriously talking performance on a test that checks if a string is present in a 100 bytes string?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

😈

@stefanoborini stefanoborini merged commit 9a921c7 into master Jul 27, 2016
@stefanoborini stefanoborini deleted the enh-default-do-not-require-remoteappmanager-file-config branch July 27, 2016 15:20
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