selenium tests, only works in VB I think, but the tests work at least #47

perpetualglow · 2018-05-15T17:35:33Z

No description provided.

jonathanvdc

Looks like a promising start! 👍 Just needs some polishing before it can get merged in. See my remarks for a list of things that can be improved.

General remark: add .vs/ to your .gitignore and commit a deletion of all files in selenium/.vs/. These Visual Studio files are personal. You don't want to share them.

Also: CI build seems to be failing for this PR. Don't know why because I haven't checked it out yet. Will do that later. You're welcome to take a look at that in the meantime.

jonathanvdc · 2018-05-16T07:34:02Z

package.json

@@ -17,6 +17,7 @@
    "jsonld": "^0.4.5",
    "material-ui": "^0.20.0",
    "n3": "^0.11.2",
+    "nuget": "^2.0.1",


Is this actually useful or is this just dead code?

Installing NuGet as a separate global dependency shouldn't be a problem at all for people who already installed a C# compiler and an MSBuild-like build system. But maybe you've got a different perspective on that and this line is warranted anyway.

Regardless, please don't put NuGet in the package.json's "dependencies"—if you really do need it, put it in the "devDependencies".

jonathanvdc · 2018-05-16T07:42:27Z

selenium/Program.cs


        public static int Main(string[] args)
        {
            // Acquire a log for printing output.
-            var rawLog = new RecordingLog(TerminalLog.Acquire());
+            var rawLog = new RecordingLog();


Why'd you change this? This'll secretly eat up all error messages, making it impossible to tell what happened when something went wrong. The tests will still fail by exiting with a nonzero exit code, but that will be obscured now.

jonathanvdc · 2018-05-16T07:44:43Z

selenium/Program.cs

@@ -78,7 +78,8 @@ public static int Main(string[] args)
                return 0;
            }

-            string testUrl = parsedOptions.GetValue<string>(Options.Url);
+            //string testUrl = parsedOptions.GetValue<string>(Options.Url);
+            string testUrl = "http://192.168.1.12:5000";


Please change this back. You can specify http://192.168.1.12:5000 as a command-line argument in the project configuration. Here's a relevant StackOverflow question.

jonathanvdc · 2018-05-16T07:47:56Z

selenium/Tests/SanityChecks.cs

        /// <summary>
        /// A list of all sanity check tests.
        /// </summary>
-        public static readonly IEnumerable<TestCase> All =
+        public static readonly ICollection<TestCase> All =


Why'd you change this to ICollection<TestCase>? Shared mutable collections are kind of bad. Consider using IReadOnlyCollection<TestCase> or even IReadOnlyList<TestCase>.

jonathanvdc · 2018-05-16T07:48:16Z

selenium/Tests/UploadFileTests.cs

+                    Assert.IsTrue(dataElement.Text == "ex:Alice");
+                });
+
+        public static readonly ICollection<TestCase> All =


Why'd you use ICollection<TestCase> here? Shared mutable collections are kind of bad. Consider using IReadOnlyCollection<TestCase> or even IReadOnlyList<TestCase>.

jonathanvdc · 2018-05-16T07:57:17Z

selenium/Program.cs

@@ -25,12 +25,12 @@ public static class Program
        /// <summary>
        /// A sequence of all test cases to run.
        /// </summary>
-        private static readonly IEnumerable<TestCase> TestCases = SanityChecks.All;
+        private static ICollection<TestCase> TestCases = UploadFileTests.All;


Why'd you use ICollection<TestCase> here? Shared mutable collections are kind of bad. Consider using IReadOnlyCollection<TestCase> or even IReadOnlyList<TestCase>. Or better yet: IEnumerable<TestCase>, as before.

Also, why exclude the sanity check tests? You can include them like so:

using System.Linq; // ... private static readonly IEnumerable<TestCase> TestCases = SanityChecks.All .Concat(UploadFileTests.All);

You might also want to include all the other tests here.

jonathanvdc · 2018-05-16T08:00:11Z

selenium/Tests/EditGraphTests.cs

+
+namespace SeleniumTests.Tests
+{
+    class EditGraphTests


These test classes contain public static members only. Consider making the classes themselves public static as well.

jonathanvdc · 2018-05-16T08:01:40Z

selenium/Tests/UploadFileTests.cs

+            driver.Manage().Timeouts().ImplicitWait = TimeSpan.FromSeconds(10);
+            //Log in
+            driver.Login();
+            String currentPath = System.IO.Directory.GetParent(System.IO.Directory.GetCurrentDirectory()).Parent.FullName;


Why not insert a using System.IO; directive at the top of the file and drop the qualifiers here? Directory.GetParent is just as clear as System.IO.Directory.GetParent and a lot shorter.

jonathanvdc · 2018-05-16T08:04:44Z

selenium/Tests/GraphTests.cs

+           String currentPath = System.IO.Directory.GetParent(System.IO.Directory.GetCurrentDirectory()).Parent.FullName;
+           String path = currentPath + @"\testfiles\demo_shacl.ttl";
+            //Open a SHACL file
+            driver.OpenFile(path);


I see the following getting duplicated over and over again.

String currentPath = System.IO.Directory.GetParent(System.IO.Directory.GetCurrentDirectory()).Parent.FullName; String path = currentPath + @"\testfiles\demo_shacl.ttl"; // Open a SHACL file driver.OpenFile(path);

You might want to refactor that pattern into a specialized (extension?) method, so you can just call driver.OpenTestFile("demo_shacl.ttl"); Also see my comment below w.r.t. the Windows-style paths: you should use Path.Combine instead.

jonathanvdc · 2018-05-16T08:05:34Z

selenium/Tests/Help.cs

+
+namespace SeleniumTests.Tests
+{
+    static class Help


Maybe rename this class to WebDriverHelpers to better express what's going on here.

…ACLed into JonasBaes

selenium tests, only works in VB I think, but the tests work at least

739f3b0

jonathanvdc requested changes May 16, 2018

View reviewed changes

perpetualglow added 2 commits May 20, 2018 14:45

further tests and implemented remarks

1635889

further tests, merging with master

bd8b63d

dubious-developments deleted a comment May 20, 2018

conformance tests

936b7cd

dubious-developments deleted a comment May 21, 2018

perpetualglow added 2 commits May 22, 2018 14:18

Merge branch 'master' of https://github.com/dubious-developments/UnSH…

26642e5

…ACLed into JonasBaes

merge with master again'

8de9d5c

dubious-developments deleted a comment May 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

selenium tests, only works in VB I think, but the tests work at least #47

selenium tests, only works in VB I think, but the tests work at least #47

perpetualglow commented May 15, 2018

jonathanvdc left a comment

jonathanvdc May 16, 2018

jonathanvdc May 16, 2018

jonathanvdc May 16, 2018

jonathanvdc May 16, 2018

jonathanvdc May 16, 2018

jonathanvdc May 16, 2018

jonathanvdc May 16, 2018

jonathanvdc May 16, 2018

jonathanvdc May 16, 2018

jonathanvdc May 16, 2018

jonathanvdc May 16, 2018

selenium tests, only works in VB I think, but the tests work at least #47

Are you sure you want to change the base?

selenium tests, only works in VB I think, but the tests work at least #47

Conversation

perpetualglow commented May 15, 2018

jonathanvdc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment